CODEWITHSUNDEEP
c#multithreadinglockingconditional-statements
user3325976
user3325976

Reputation: 819

C# thread accesed to if block, whose condition returns false

This block of code is being accessed by many threads 

    // All code is from same class

    public void ExecuteCommand(IAsciiCommand command, IAsciiCommandSynchronousResponder responder)
    {
        lock (commander)
        {
            if (commander.IsConnected)
            {
                commander.ExecuteCommand(command, responder);
            }
        }
    }

    public void Disconnect()
    {
        var tmp = commander.IsConnected;
        commander.Disconnect();
        if (commander.IsConnected != tmp && !commander.IsConnected)
        {
            OnPropertyChanged("IsConnected");
        }
    }

And eventually i get this: Error

How is this possible, that thread accessed into if statement, whose condition returns false? How can i fix it?

Upvotes: 2

Views: 252

Answers (3)

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726559

This is happening because the check and the call lack atomicity. Here is a sequence of events that could lead to an exception:

  • Two threads, A and B, are reaching the condition at the same time
  • Thread A checks the condition, which returns true, so it enters the if block
  • At the same time, thread scheduler decides that thread A has exhausted its time slot, and suspends it
  • Thread B calls Disconnect
  • Thread scheduler resumes thread A, which is inside the if condition. However, the command is no longer connected
  • This causes the exception

You can fix it by locking commander inside Disconnect().

public void Disconnect()
{
    bool doEvent;
    lock(commander) {
        var tmp = commander.IsConnected;
        commander.Disconnect();
        doEvent = (commander.IsConnected != tmp && !commander.IsConnected)
    }
    // Run OnPropertyChanged outside the locked context 
    if (doEvent)
    {
        OnPropertyChanged("IsConnected");
    }
}

Upvotes: 2

vgru
vgru

Reputation: 51214

If you are not locking while disconnecting, it's entirely possible to get a race condition. The basic solution is to add a lock inside the Disconnect method:

public void Disconnect()
{
    lock (commander)
    {
        var tmp = commander.IsConnected;
        commander.Disconnect();
        if (commander.IsConnected != tmp && !commander.IsConnected)
            OnPropertyChanged("IsConnected");
    }
}

Upvotes: 1

Philip Pittle
Philip Pittle

Reputation: 12295

You need to lock on a static object. Right now you're creating separate locks based on the object your are working with (commander). Try this:

public class WhatEverClassHasTheExecuteCommandMethod
{
     private static object _lock = new object();

     public void ExecuteCommand(IAsciiCommand command, IAsciiCommandSynchronousResponder responder)
     {
          lock (_lock)
              if (commander.IsConnected)                  
                 commander.ExecuteCommand(command, responder);
     }
}

Upvotes: 1

Related Questions