Lock usage of all methods

Question

I have a method that is accessed from multiple threads at the same time and I want to make sure that only 1 thread can be inside of a body of any method.

Can this code be refactored to something more generic? (Apart from Locking inside the State property?

public class StateManager : IStateManager
{
    private readonly object _lock = new object();
    public Guid? GetInfo1()
    {
        lock (_lock)
        {
            return State.Info1;
        }
    }

    public void SetInfo1(Guid guid)
    {
        lock (_lock)
        {
            State.Info1 = guid;
        }
    }

    public Guid? GetInfo2()
    {
        lock (_lock)
        {
            return State.Info2;
        }
    }

    public void SetInfo2(Guid guid)
    {
        lock (_lock)
        {
            State.Info2 = guid;
        }
    }
}

Answer 1

I would like to add what I ended up putting together, using some of the ideas presented by Matt and Abhinav in order to generalize this and make it as seamless as possible to implement.

private static readonly object Lock = new object();

public static void ExecuteMethodThreadSafe<T>(this T @object, Action<T> method) {
    lock (Lock) {
        method(@object);
    }
}

public static TResult ExecuteMethodThreadSafe<T, TResult>(this T @object, Func<T, TResult> method) {
    lock (Lock) {
        return method(@object);
    }
}

Which can then be extended in ways like this (if you want):

private static readonly Random Random = new Random();

public static T GetRandom<T>(Func<Random, T> method) => Random.ExecuteMethodThreadSafe(method);

And then when implemented could look something like this:

var bounds = new Collection<int>();
bounds.ExecuteMethodThreadSafe(list => list.Add(15)); // using the base method
int x = GetRandom(random => random.Next(-10, bounds[0])); // using the extended method
int y = GetRandom(random => random.Next(bounds[0])); // works with any method overload

Answer 2

Maybe something like:

private void LockAndExecute(Action action)
{
    lock (_lock)
    {
        action();
    }
}

Then your methods might look like this:

public void DoSomething()
{
    LockAndExecute(() => Console.WriteLine("DoSomething") );
}

public int GetSomething()
{
    int i = 0;
    LockAndExecute(() => i = 1);
    return i;
}

I'm not sure that's really saving you very much however and return values are a bit of a pain.

Although you could work around that by adding another method like this:

private T LockAndExecute<T>(Func<T> function)
{
    lock (_lock)
    {
        return function();
    }
}

So now my GetSomething method is a lot cleaner:

public int GetSomething()
{
    return LockAndExecute(() => 1 );
}

Again, not sure you are gaining much in terms of less typing, but at least you know every call is locking on the same object.

While your gains may be pretty minimal in the case where all you need to do is lock, I could imagine a case where you had a bunch of methods something like this:

public void DoSomething()
{
    // check some preconditions
    // maybe do some logging
    try
    {
        // do actual work here
    }
    catch (SomeException e)
    {
        // do some error handling
    }
}

In that case, extracting all the precondition checking and error handling into one place could be pretty useful:

private void CheckExecuteAndHandleErrors(Action action)
{
    // preconditions
    // logging
    try
    {
        action();
    }
    catch (SomeException e)
    {
        // handle errors
    }
}

Answer 3

Using Action or Function Delegate.

Creating a method like

public T ExecuteMethodThreadSafe<T>(Func<T> MethodToExecute)
    {
        lock (_lock)
        {
            MethodToExecute.Invoke();
        }
    }

and using it like

public T GetInfo2(Guid guid)
        {
            return ExecuteMethodThreadSafe(() => State.Info2);
        }