replicator.org ... incoherent code

To make a class thread-safe, there are a number of options:

1. Protect each method by a lock() scope or attribute it by [MethodImpl(MethodImplOptions.Synchronized)].
2. Create a proxy class by hand that uses a lock() scope for each method and forwards them to the unsynchronized target.
3. Dynamically create a “synchronizing” proxy class by using a “Dynamic Proxy” library.
4. Mechanically adjust the generated IL of the class to include the lock() scopes by using AOP processors like PostSharp.

1. Clearly violates the Single Responsible Principle, because it mixes synchronization concerns with the implementation of the class.
2. Requires to implement lot of stupid forwarding code.
3. Essentially fixes the problem in 2. So far the best solution. Requires a lot of library code, though.
4. Transparently integrates the locking inside the methods, essentially fixing 1. But postprocessing feels always magical and “hidden”, so it is not obvious to clients if there is synchronization required or not.

But the problem with all the solutions above is that the locking mechanism gets hidden inside the implementation and so a consistent synchronization for more than one method is not possible.

i.e, in the following simple example, it can not be guaranteed, that there is actually work to do when doTheWork() is called.

var obj = new Obj();
 
...
 
if (obj.isWorkToDo())
  obj.doTheWork();

In the meantime, some other thread may have grabbed the “work” and executed doTheWork() before.

One solution I came up, is to virtually inject code that needs to processed in the synchronization context. The resulting helper ISynchronized<T> can be applied to any class instance:

var obj  = new Obj();
var synced = obj.synchronized();
 
...
 
synced.use(instance =>
{
  if (instance.isWorkToDo())
    instance.doTheWork();
}

The advantages I can see from this approach are:

1. It is obvious to clients that certain classes are synchronized. Instead of the class or interface reference, an ISynchronized<T> is passed around.
2. The implementation of the class does not need to be changed, old classes can be reused in a thread-safe context.
3. Multiple methods can be called in one lock() scope.

The disadvantage is that client code can not use the original class’s interface, it’s always ISynchronized<T>.

I personally think that the advantages clearly outweigh the disadvantages. Mainly because clients want to access shared instances in a transactional way so they don’t need to care about state fluctuations caused by other threads.

Below the current implementation of the synchronization helper:

public interface ISynchronized<T>
	where T : class
{
	void use(Action<T> action);
	bool tryUse(Action<T> action);
}
 
sealed class Synchronized<T> : ISynchronized<T>
	where T : class
{
	readonly T _v;
 
	public Synchronized(T v)
	{
		_v = v;
	}
 
	public void use(Action<T> action)
	{
		lock (_v)
			action(_v);
	}
 
	public bool tryUse(Action<T> action)
	{
		if (!Monitor.TryEnter(_v))
			return false;
		try
		{
			action(_v);
		}
		finally
		{
			Monitor.Exit(_v);
		}
		return true;
	}
}
 
public static class SynchronizedExtensions
{
	public static ISynchronized<T> synchronized<T>(this T instance)
		where T : class
	{
		return new Synchronized<T>(instance);
	}
}