Concurrency issue with new Bitmap(oldBitmapInstance)

Question

The following two snippets are used concurrently by multiple threads:

Bitmap newBitmap = null;
lock (currentBitmap)
    newBitmap = new Bitmap(currentBitmap); // throws InvalidOperationException

The newBitmap object is manipulated in the meantime by using Graphics.FromImage(newBitmap) and subsequently some drawing methods of the Graphics object.

lock (currentBitmap)
    currentBitmap= new Bitmap(newBitmap);

Those are the only interactions with the currentBitmap object. I am never handing over it's reference like

Bitmap xxx = currentBitmap;

Yet on the first snippet at the Bitmap constructor it throws an InvalidOperationException. In my opinion this shouldn't be able to happen because of the lock statements. Aren't they ensuring that it's never used by two threads at once unless the reference "escapes" somehow, which it doesn't in my case?

What more do I need to do besides locking the object?

Answer 1

Note that when each time the lock statement is executed it will evaluate the current reference pointed to by the parameter you pass to it. In this case as you are overwriting the variable that is passed to lock so each time it is executed it will lock on a different reference.

You should never lock on an object that is modified within the lock statement. Also, you should never lock on a value type (as there is no global reference) nor lock on this (as you don't control which other classes can have a reference to yours and lock on it).

As a guideline you should look to lock on a dedicated object created for this purpose and in my opinion this object should also be defined as readonly.

private readonly _shared = new object();

private void SomeMethod()
{    
    lock (_shared) 
    {
         // Stuff
    }
}