My BitTorrent based P2P app is heavily multi-threaded. Most of it is in the form of asynchronous socket transfers, but some of it is my own doing – for each torrent I create a thread separate from the main GUI thread.

Recently I have run into a deadlock on a place that looked perfectly normal. As it turns out, I made a classical deadlock situation by a code like this:

// on a thread 1
lock(lockA)
{
  // do something 1
  lock(lockB)
  {
    // ...
  }
}

// on a thread 2
lock(lockB)
{
  // do something 2
  lock(lockA)
  {
    // ...
  }
}

One of the threads will grab lockA and while executing something 1 the other will grab the lockB – or the other way around.

If possible, it’s easy to avoid this kind of problem. Think about why do you have two locks – is it really necessary? Sometimes it is, because you want better granularity, but in this case I just left some code in after a significant refactoring (I know, I know, unit and/or functional tests should have caught this, but they didn’t).

If it is not really necessary, just replace two locks with one. Even better, if possible, replace 3 or more locks with one. I managed to replace 3 locks with one and if I could have, I would have avoided even that one.

This is KISS principle at its best – less code and less problems.