The provided sample code certainly leaves open a large number of questions about your particular use-case; however, I will attempt to answer the general strategy to implementing this type of problem for a multi-threaded environment.
Does the context or its data get modified in a coupled, non-atmoic way?
For example, would any of your commands do something like:
Context.Data.Item1 = "Hello"; // Setting both values is required, only
Context.Data.Item2 = "World"; // setting one would result in invalid state
Then absolutely you would need to utilize lock(...)
statements somewhere in your code. The question is where.
What is the thread-safety behavior of your nested controllers?
In the linked GIST sample code, the CommandContext
class has properties ServerController
and ServiceController
. If you are not the owner of these classes, then you must carefully check the documentation on the thread-safety of of these classes as well.
For example, if your commands running on two different threads perform calls such as:
Context.ServiceController.Commit(); // On thread A
Context.ServiceController.Rollback(); // On thread B
There is a strong possibility that these two actions cannot be invoked concurrently if the creator of the controller class was not expecting multi-threaded usage.
When to lock and what to lock on
Take the lock whenever you need to perform multiple actions that must happen completely or not at all, or when invoking long-running operations that do not expect concurrent access. Release the lock as soon as possible.
Also, locks should only be taken on read-only or constant properties or fields. So before you do something like:
lock(Context.Data)
{
// Manipulate data sub-properties here
}
Remember that it is possible to swap out the object that Data
is pointing to. The safest implementation is to provide a special locking objects:
internal readonly object dataSyncRoot = new object();
internal readonly object serviceSyncRoot = new object();
internal readonly object serverSyncRoot = new object();
for each sub-object that requires exclusive access and use:
lock(Context.dataSyncRoot)
{
// Manipulate data sub-properties here
}
There is no magic bullet on when and where to do the locks, but in general, the higher up in the call stack you put them, the simpler and safer your code will probably be, at the expense of performance - since both threads cannot execute simultaneously anymore. The further down you place them, the more concurrent your code will be, but also more expense.
Aside: there is almost no performance penalty for the actual taking and releasing of the lock, so no need to worry about that.