Category: CYSDL: solutions

Can You Spot the Deadlock 2?

Hello dear readers and coders

Welcome to the second ‘Can You Spot the Deadlock?’ trivia. I surely hope you had fun with the first one. Yeah, I know, it was kinda easy. So today, I raise the bar a bit and bring you one of my favorites: the transactional subscription/un subscription pattern.

The code compiles but it will not run, as I did not show the scaffolding code. Suffice to say that there is a notification mechanism that lives in its own thread(s) and some client code logic that rely on the main(starting) thread.

So:

  • What is the issue here?
  • Where does it occur?
  • Can you fix it ? if yes how, if now, why

Problem is now closed, please find the solution

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace SpotTheDeadLock2
{
///
/// Implements aynchronous notification
///
public class EventSource
{
    private EventHandler _eventHandler;
    ///
    /// notification event
    ///

    public event EventHandler EventOccured
    {
        add {
            // lock to secure subscription
            lock (this){
                _eventHandler += value;
            }
        }
        remove {
            // lock to secure unsubscription
            lock (this) {
                _eventHandler -= value;
            }
        }
    }
    // implementation of notification
    private void TriggerEvent() {
        lock (this) {
            if (_eventHandler != null) {
                _eventHandler(this, new EventArgs());
            }
        }
    }
}

///
/// client implementation
///
public class Client: IDisposable
{
    private EventSource _source;
    ///
    /// Simple constructor
    ///
    ///data source to subsribe to
    public Client(EventSource source) {
        _source = source;
        _source.EventOccured += OnEvent;
    }

    ///
    /// Method in charge of processing events
    ///
    private void OnEvent(object sender, EventArgs arguments) {
        // use lock as we do not have control of calling thread
       lock (this) {
           // do my job
           ...
       }
    }
    // unsubscribe on clean up
    public void Dispose() {
    // use lock to ensure no processing is in progress
        lock (this) {
            _source.EventOccured -= OnEvent;
        }
    }
}
}

CYSDL 1: Solution

As promised, here is the answer to my question
Here the deadlock manifests itself as the main thread stuck on one of the Thread.Join calls and the associated thread’s (_firstRunner or _secondRunner) Monitor.Wait(_synchro) call.

The problem lies with the Monitor.Pulse(_synchro) call, because it wakes only one thread waiting allowing it to end properly. The other thread is still waiting for a signal that will never happen. And ultimately, the main thread is waiting for both threads to end, and we already know that it will not happen.

How to fix: you need to replace Monitor.Pulse(_synchro) by Monitor.PulseAll(_synchro).

This example reminds us that deadlocks do not systematicaly occurs on lock statement and that one must be careful regarding thread termination. Plus, here the problem does not relate to the classical ordering of resource acquisition.

The only random aspect was which of the two runner threads would be locked.

It also raises concern about when and why someone has to worry about using Pulse or PulseAll.

How can it be improved: there is a call to Thread.Sleep that I did place just to make sure that both runner thread where waiting for the signal. This is never the proper way to make a rendez-vous point between threads. But this is neither a trivial matter and adequate rendez-vous code would add a significant chunck of code.

Rendez-vous will probably a topic for another exercise.

How did you fare in this exercise

PS: This an automated post

Can You Spot the DeadLock?

Hi everyoneI was considering starting to put some code in this blog, but was lacking the proper motivation. But I found one: trivia game!

So welcome to the first instalment of

Can You Spot the Deadlock?

Rules are simple: I publish some code which exhibits some concurrency related problem and ask some questions about it such as:

  • What concurrency issue(s) is(are) present in the code? (deadlock, race condition, livelock…)
  • Where does it occurs or what are the impacts?
  • How can it be fixed

Please post your answer(s) in the comment section, I will provide the expected answers one week after first submission. Of course, debugging is not allowed here.
So without further ado, here is the first one, quite easy for a starter.

Where is the deadlock?

Which single line needs to be changed to fix this?

Problem is now closed, please see the solution.

class Program
{
  /// Main function
  static void Main(string[] args)
  {
    DeadLock1 sample1 = new DeadLock1();
    sample1.Start();
  }
}
/// First instance of 'Can you Spot the DeadLock?
/// Let's start easy to put things in motion
internal class DeadLock1
{
  private Thread _firstRunner;
  private Thread _secondRunner;
  private bool _stopSignal = false;
  private readonly object _synchro = new object();

  public DeadLock1()
  {
    _firstRunner = new Thread(FirstRunner);
    _secondRunner = new Thread(SecondRunner);
  }

  // start your engines
  public void Start()
  {
    _firstRunner.Start();
    _secondRunner.Start();
    Thread.Sleep(100);
    lock (_synchro)
    {
      _stopSignal = true;
      Monitor.Pulse(_synchro);
    }
    _firstRunner.Join();
    _secondRunner.Join();
  }
  // first thread logic
  private void FirstRunner()
  {
     lock (_synchro)
     {
       if (!_stopSignal)
       {
         Monitor.Wait(_synchro);
       }
     }
   }
  // second thread logic
  private void SecondRunner()
  {
    lock (_synchro)
    {
      if (!_stopSignal)
      {
        Monitor.Wait(_synchro);
      }
    }
  }
}

Several quality comments can be made about this code, but please focus on the multithreading issues. Bonus points if the second design weakness is identified!

PS: I’ll wait a few days before approving comments not to disclose the solution. I already have at least one good answer (but not perfect) and one wrong one. Keep commenting!

edited to explicitly initialize _stopSignal for clarity.