Approach for synchronizing access to a resource in C# using the IDisposable pattern

Question

I'm thinking about an approach to use the IDisposable pattern to synchonize/coordinate access to a shared resource.

Here's my code so far (easy to run with LinqPad):

#define WITH_CONSOLE_LOG
//better undefine WITH_CONSOLE_LOG when testing long loops

public abstract class SynchronizedAccessBase
{
    private readonly object syncObj = new();

    private class AccessToken : IDisposable
    {
        private SynchronizedAccessBase parent;
        private bool didDispose;

        public AccessToken(SynchronizedAccessBase parent)
        {
            this.parent = parent;
        }

        protected virtual void Dispose(bool disposing)
        {
            if (!this.didDispose)
            {
                Monitor.Exit(this.parent.syncObj);
#if WITH_CONSOLE_LOG
                Console.WriteLine("Monitor.Exit by Thread=" + Thread.CurrentThread.ManagedThreadId);
#endif
                if (disposing)
                {
                    //nothing specific here
                }
                this.didDispose = true;
            }
        }
        ~AccessToken()
        {
            this.Dispose(disposing: false);
        }

        void IDisposable.Dispose()
        {
            Dispose(disposing: true);
            GC.SuppressFinalize(this);
        }
    }

    public IDisposable WantAccess()
    {
        Monitor.Enter(this.syncObj);
#if WITH_CONSOLE_LOG
        Console.WriteLine("Monitor.Enter by Thread=" + Thread.CurrentThread.ManagedThreadId);
#endif
        return new AccessToken(this);
    }
}

public class MyResource : SynchronizedAccessBase
{
    public int Value;
}

private MyResource TheResource;

private void MyMethod()
{
    using var token = TheResource.WantAccess(); //comment out this line to see the unsynced behavior
#if WITH_CONSOLE_LOG    
    Console.WriteLine("Inc'ing Value by Thread=" + Thread.CurrentThread.ManagedThreadId);
#endif
    TheResource.Value++;
}

void Main()
{
    this.TheResource = new MyResource();
    
    var inc_tasks = new Task[10];
    for (int i = 0; i < inc_tasks.Length; i++)
        inc_tasks[i] = Task.Run(() =>
        {
            for (int loop = 1; loop <= 100; loop++)
                MyMethod();
        });

    Task.WaitAll(inc_tasks);

    Console.WriteLine("End of Main() with Value==" + TheResource.Value);
}

What I'm trying to achieve is to use the C# "using" statement at the top of a method (or somewhere in the middle, who cares) before the (exclusive) access to the shared resource and have the IDisposable mechanism automatically ending the exclusive access.

Under the hood the Monitor class is used for that.

The desired advantage is that no indented { code block } is required. Just a using... line and that's it. See MyMethod() in the sample above.

This seems to work quite fine. The final result of all inc's is as expected, even with long loops, and wrong if I remove the using.. statement from MyMethod.

However, do you think I can trust this solution? Is .Dispose of the token really, really always called when leaving MyMethod, even in case of an exception? Other pitfalls?

Thanks!

Yes, that's a valid approach. You can see an example of a similar use [here](https://stackoverflow.com/questions/31138179/asynchronous-locking-based-on-a-key/31194647#31194647) (the `Releaser` class). For synchronous locking, it should probably be more efficient to use a struct instead of a class as the type of the `Releaser`. — Theodor Zoulias, Mar 19 '21 at 15:32
I tried with struct instead of class for AccessToken. Yes, that's significant faster (30-40%) — KarloX, Mar 19 '21 at 16:34
If you decide to make it a struct, you should consider changing the return type of the `WantAccess` method from `IDisposable` to `AccessToken` (would require making the `AccessToken` public). This way the struct would not be boxed by the `using` statement, resulting to fewer allocations and less work for the garbage collector. — Theodor Zoulias, Mar 19 '21 at 16:45
@TheodorZoulias, I tried that, too, but there's no performance gain I can measure — KarloX, Mar 19 '21 at 16:54
Maybe the `protected virtual`, the `~AccessToken()` finalizer and the `GC.SuppressFinalize(this);` call slow things down. Do your really need this stuff? Do you anticipate a scenario where you'll have to inherit from the `AccessToken` type? — Theodor Zoulias, Mar 19 '21 at 17:01
@TheodorZoulias, after changing from class to struct these elements are gone anyway. — KarloX, Mar 19 '21 at 23:12
KarloX yeap, you are right. Still surprised that you get the same performance using interfaces as with using concrete types. — Theodor Zoulias, Mar 19 '21 at 23:42

Servy · Accepted Answer · 2021-03-19T15:45:22.187

1

Your code has the potential to hold onto the lock outside of the scope of the critical section if an exception is thrown after you enter the monitor in WantAccess and before the returned value is assigned to the variable in the using block. lock has no potential for doing the same due to the way the transformation is done.

lock is specially designed for exactly this problem, and is very finely tuned to do exactly what you would want it to do when solving this kind of problem. You should use the right tool for the job.

edited Mar 19 '21 at 15:45

answered Mar 19 '21 at 15:32

Servy

202,030
26
332
449

(holding lock if exception is thrown) Yes that is a good spot and the kind of bug that is a nightmare to repeat and find. You could fix that with some exception handling. – mikelegg Mar 19 '21 at 16:20
@Servy I see your point. However I can't think of a reason why such an exception could ever occur after Monitor.Enter() has successfully been called and before the result of WantAccess is assigned to the token variable. Ok, an out-of-memory exception could occur on new AccessToken(). So one could better prepare that instance before the Monitor.Enter() call. Other ideas what could go wrong (in real-world)? – KarloX Mar 19 '21 at 16:21
@mikelegg Sadly you cannot. You could handle some of the cases, but not all. What `lock` does, which you would need to replicate to cover every case it does, is to call Monitor.TryEnter inside of a try/finally so the finally can check if the lock was taken and release it. No `using` implementation is capable of replicating that. – Servy Mar 19 '21 at 16:22
@KarloX Well it only takes one, and you mentioned one, so that's enough to be a dealbreaker right there. On top of that you have stack overflows and thread abort exceptions being possible. Finally, you're accessing standard output, which can throw for a wide variety of reasons. – Servy Mar 19 '21 at 16:24
@Servy, ok standard ouput wouldn't make into production code of course. Thread abort is a valid point. Stack overflow on just returning a result and assigning it to a declared local variable, hmm, I don't known, maybe. Thanks – KarloX Mar 19 '21 at 16:29
@KarloX Having bugs in some builds and not others is also *really bad* and a route to tons of wasted time and confusion. – Servy Mar 19 '21 at 16:31
@KarloX It's *not* just returning a result. It's constructing a new object. The SO exception you could prevent by constructing the object before taking out the lock, but that still leaves the other problems. – Servy Mar 19 '21 at 16:36
@Servy, yeah I mean after implementing what I said in my earlier comment "_So one could better prepare that instance before the Monitor.Enter() call_". But you are right of course. – KarloX Mar 19 '21 at 16:42

score 0 · Answer 2 · answered Mar 19 '21 at 15:20

I don't see anything wrong with this at all. I have used the using pattern in a similar way before without issue.

As a simple example, consider the pattern for using a database connection. Underneath there is a connection pool, creating a new connection takes from the pool (possibly waiting) and disposing releases back to the pool. This is the same as you have, all be it with a pool of 1 item.

I don't know if the effort is worth not just using a simple lock(){} pattern if the resource is only shared within 1 process. That would be more 'conventional'. But only you can answer that.

Approach for synchronizing access to a resource in C# using the IDisposable pattern

2 Answers2