mysayasan
mysayasan

Reputation: 39

The proper callback from threading mode in c#

I'm using threads to connect to multiple clients (PLCs) from my program. Program will send data and receive response from the PLC(s).. The problem im having is, when in debugging mode, (toggle breakpoint) one step at a time..the program work pretty much ok!, with the ID received confirming that it's coming from one of the thread.. but if I just debug without toggling any breakpoint, the response event will receive the same ID, although on different thread.. what could be wrong...

Debugging mode with breakpoint:

Debugging mode without breakpoint:

Below is my code

Start Request:

private void StartRequest()
{            
    foreach (ModbusTCP work in works)
    {
        work.Connect();
        Thread.Sleep(1000);
        if (work.Connected)
        {
            try
            {
                Thread thread = new Thread(new ThreadStart(() => work.StartReadHoldingRegister())) {
                    Name = ((ReadHoldingRegisterParam)work.SetReadHoldingRegisterParam).id.ToString(),
                    IsBackground = true
                };
                work.OnResponseEvent += new EventHandler<ModbusTCP.ResponseEventArgs>(modbus_OnResponseEvent);
                work.OnExceptionEvent += new EventHandler<ModbusTCP.ExceptionEventArgs>(modbus_OnExceptionEvent);                                            
                thread.Start();
                threads.Add(thread);
            }
            catch (ThreadStateException ex)
            {
                MessageBox.Show(ex.Message);
            }
        }
        else
            work.Disconnect();
    }            
}

Response Event

private void modbus_OnResponseEvent(object sender, ModbusTCP.ResponseEventArgs e)
{
    lock (lockingObject)
    {
        if (e.data.Length > 0)
        {
            this.Dispatcher.BeginInvoke(new Action(() =>
            {
                AddRow(RcvDataGrid, new PLCPacket() {
                    PLCId = e.id.ToString(),
                    PLCIp = "Test",
                    PLCTime = DateTime.Now.ToString("yyyy-MM-dd hh:mm:ss tt"),
                    PLCData = ""
                });
            }));
        }
    }
}

Upvotes: 3

Views: 102

Answers (2)

Remus Rusanu
Remus Rusanu

Reputation: 294227

A side comment:

lock (lockingObject)
{
    if (e.data.Length > 0)
    {
        this.Dispatcher.BeginInvoke(new Action(() =>
        {

This code is very unlikely to be correct. Here you are obtaining a lock in the original thread and then submitting a new action, async. The lock is scoped to the current method, and thus will be released as soon as the BeginInvoke call returns, not during the action itself. The only operations actually guarded by the lock is the e.data.Length check, which operates on a parameter (not shared) state and thus does not need protection.

It would make more sense to place the lock inside the action, but the action is always executed on the main thread and thus is unlikely to actually need protection (because is, basically, single threaded). Is difficult to guess exactly what you're trying to achieve w/o seeing the whole code, but that lock(lockingObject) is very unlikely to be necessary, or useful.

Upvotes: 0

rene
rene

Reputation: 42424

Your variable work is shared among the threads. Once a thread is executed it takes whatever value your variable work has. That depends how quick each thread is processed. When you step through your code with a debugger you don't experience that.

If you capture the value before the anonymous method you should be fine:

 try
 {
      // capture the current value of the loop variable
      ModbusTCP localWork = work;
      // so the anonymous method uses the reference in localWork
      // instead of whatever value work has, which can be anywhere
      // the future, worst case after your loop is finished, where
      // work would hold the last value of the loop, and then
      // start all threads with that value.
      Thread thread = new Thread(
           new ThreadStart(
               () => localWork.StartReadHoldingRegister())) 
               { 
                  Name = ((ReadHoldingRegisterParam) localWork.SetReadHoldingRegisterParam).id.ToString(), 
                  IsBackground = true };
               });
       localWork.OnResponseEvent += new EventHandler<ModbusTCP.ResponseEventArgs>(modbus_OnResponseEvent);
       localWork.OnExceptionEvent += new EventHandler<ModbusTCP.ExceptionEventArgs>(modbus_OnExceptionEvent);                                            

Upvotes: 4

Related Questions