Deadlocking lock() method

Question

I'm facing a deadlock, my code structure is similar to this:

private delegate void UpdateControlDelegate(string value);

public void UpdateControl(string value)
{
    if (txtAddress.InvokeRequired)
    {
        txtAddress.Invoke(new UpdateControlDelegate(UpdateControl), value);
    }
    else
    {
        txtAddress.Text = value; // This is in GroupBox1
        txtValue.Text = value; // This is in GroupBox2
    }
}

class ThreadHandler
{
    List<string> _list = new List<string>();
    object _criticalSection = new object();

    public ThreadHandler()
    {
        new Thread(new ThreadStart(Run)).Start();
    }

    public static ThreadHandler _threadHandler = null;

    public static ThreadHandler GetThreadHandler()
    {
        if (_threadHandler == null)
        {
            _threadHandler = new ThreadHandler();
        }

        return _threadHandler;
    }

    public void Run()
    {
        while (true)
        {
            // some code

            lock (_criticalSection)
            {
                foreach (string str in _list)
                {
                    // some Code
                }
            }

            // some code
            Thread.Sleep(SomeTime);
        }
    }

    public void AddItem(string item)
    {
        lock (_criticalSection)
        {
            _list.Add(item);
        }
    }

    public void RemoveItem(string item)
    {
        lock (_criticalSection)
        {
            _list.Remove(item);
        }
    }

}

But using the same code, I just modified the UpdateControl method like this:

private delegate void UpdateControlDelegate(string value);

public void UpdateControl(string value)
{
    if (InvokeRequired)
    {
        BeginInvoke(new UpdateControlDelegate(UpdateControl), value);
    }
    else
    {
        txtAddress.Text = value; // This is in GroupBox1
        txtValue.Text = value; // This is in GroupBox2
    }
}

This is working fine. What is the problem?

Answer 1

Here is my code,

public class ValueReader
{
    List<IDataReader> _list = new List<IDataReader>();

    object _criticalSection = new object();

      public ValueReader()
      {
        //Nothign here
      }

      public void Attach(IDataReader reader)
      {
            lock(_criticalSection)
            {
                _list.Add(reader);
            }
      }

      public void Detach(IDataReader reader)
      {
            lock(_criticalSection)
            {
                _list.Remove(reader);
            }
      }

      public void Notify(string value)
      {
            lock(_criticalSection)
            {
                foreach(IDataReader reader in _list)
                {
                    reader.Update(value);
                }
            }
        }

      public void Start()
      {
            new Thread(new ThreadStart(Run)).Start();
      }


      private void Run()
      {
            while(true)
            {

                //generate value
                Notify(value);

                Thread.Sleep(5000);

            }
      } 

}




public interface IDataReader
{
    void UpdateControls(string value);
}

public class FirstClass : IDataReader
{

    ....
    ......  
    ValueReader _reader = null;

    public FirstClass()
    {

        _reader = new ValueReader();
              _reader.Start();
        _reader.Attach(this);

    }

    private void AddToSmartClient()
    {
        // _reader has added to SmartClient's WorkItem
    }


    private delegate void UpdateControlDelegate(string value);

    public void UpdateControl(string value)
    {
        if(txtAddress.InvokeRequired)
        {
            txtAddress.Invoke(new UpdateControlDelegate(UpdateControl), value);
        }
        else
        {
            txtAddress.Text = value;
            txtValue.Text = value;
        }
    }

}


public class SecondClass : IDataReader
{
        ....
    ......
    ValueReader _reader = null;

    public void SecondClass()
    {
        _reader = ReadFromSmartClient();
        _reader.Attach(this);
    }

    private ValueReader ReadFromSmartClient()
    {
        reader = //Get from SmartClient's Workitem.
        return reader
    }

    private delegate void UpdateControlDelegate(string value);

    public void UpdateControl(string value)
    {
        if(InvokeRequired)
        {
            BeginInvoke(new UpdateControlDelegate(UpdateControl), value);
        }
        else
        {
            control1.Text = value;
            control2.Text = value;
        }
    }

}

I invokes only FirstClass for some time. In this case its working fine. After some time i invoked the Second class, at this time while calling Attach from secondClass the application is hanging.(i monitered that its comming until Attach methods's lock(_criticalSection).

After some time i converter the Update control in the Frist Class as follow

     public void UpdateControl(string value)
{
    if(InvokeRequired)
    {
        BeginInvoke(new UpdateControlDelegate(UpdateControl), value);
    }
    else
    {
        txtAddress.Text = value;
        txtValue.Text = value;
    }
}

This is working well after invoking the SecondClass also. Why its happening?

Answer 2

Are you calling AddItem and RemoveItem from the main thread while calling UpdateControl from the worker thread? That will cause deadlocks.

Answer 3

The problem is almost certainly that you're acquiring the lock within a background thread, then calling Control.Invoke, and invoking a delegate (on the UI thread) which tries to acquire the same lock. It can't do that, because the other thread holds the lock - and will keep holding the lock while it waits for the UI operation to complete.

Admittedly there's no locking within the UpdateControl method you've posted, but I suspect that's not the complete code - and you haven't shown where you're using AddItem or RemoveItem.

I note that GetThreadHandler() isn't thread-safe, by the way - that looks like a bug to me...