Cargo-cult programming: locking in System.Object

Question

I think I developed a habit of culinary cult:

Whenever I need to make a class thread safe, for example a class that has a dictionary or list (which is fully encapsulated: it has never been accessed directly and changed only by member methods of my class) I create two objects, for example:

public static class Recorder { private static readonly Object _devicesLock = new Object(); private static readonly Dictionary<String,DeviceRecordings> _devices; static Recorder() { _devices = new Dictionary<String,DeviceRecordings>(); WaveInCapabilities[] devices = AudioManager.GetInDevices(); foreach(WaveInCapabilities device in devices) { _devices.Add( device.ProductName, new DeviceRecordings( device.ProductName ) ); } }//cctor // For now, only support a single device. public static DeviceRecordings GetRecordings(String deviceName) { lock( _devicesLock ) { if( !_devices.ContainsKey( deviceName ) ) { return null; } return _devices[ deviceName ]; } }//GetRecordings }//class 

In this case, I complete all operations on _devices in the lock( _devicesLock ) { block. I begin to think, if necessary. Why don't I just lock the dictionary directly?

Answer 1

In your case, locking the dictionary will be fine, as it is closed. You should still carefully design your class to prevent deadlocks.

If the dictionary is the only shared resource that requires thread safety, and other parts of your code are thread safe, I would recommend using ConcurrentDictionary instead of blocking,

Answer 2

This is not necessary if you are sure that the main object is fully used inside the class. Strictly speaking, this is not necessary, even if it is a version with variable 2, it is much easier to reason:

• Someone reading the code does not need to think about the fact that the main object is ever exposed and potentially blocked by something that your class has learned.
• code with a separate object for locking is more in line with good practice.
• it will be safer if accidentally / intentionally gives out the main object