A lock statement with timeout…


Ian Griffiths comes up with an interesting way to use IDisposable and the “using“ statement to get a very of lock with timeout.


I like the approach, but there are two ways to improve it:


1) Define TimedLock as a struct instead of a class, so that there’s no heap allocation involved.


2) Implement Dispose() with a public implementation rather than a private one. If that’s the case, the compiler will call Dispose() directly, otherwise it will box to the IDisposable interface before calling Dispose().


 

Comments (25)

  1. Doug McClean says:

    I’ve done this trick before to deal with common patterns in numerous methods. Usually lock acquisition, but there are some others. Problem is it always feels like a hack since the object isn’t really disposable so much as "call-back-at-the-end-of-a-scope-able". Consideration might be given to a language feature to make this easier (or to someone writing a really good static aspect weaver…).

    While we’re on the subject, it would also be cool if you could foreach() over IEnumerators in addition to IEnumerables. This would be very useful when collections have more than one natural enumeration order.

  2. If you change TimedLock to a struct and expose Dispose() as public, will the compiler call Dispose when an instance of TimedLock goes out of scope if you forget to call Dispose() (for example, you are not using the "using" statement)?

    In Ian’s example, he uses a finalizer in debug mode to warn if someone forgets to call dispose. But with a struct, you can’t have a finalizer.

  3. Chance Gillespie says:

    Interesting idea. Something I noticed though. You already mentioned using a struct instead of a class and this may be the reason. If the monitor fails then an exception is thrown. That exception is thrown before the instance of the TimedLock class is returned which means dispose will not get called on that newly created instance (as expected) but also means that the assertion in the finalizer will always (given the GC gets to the finalizer of course) fail if the Monitor.TryEnter fails. It never gets a chance to call GC.SuppressFinalize (). That’s fine if you want to assert any time a lock fails but doesn’t work as an assertion to determine if we forget to call dispose on the TimedLock. Doesn’t effect a release build, of course, cause it’s wrapped in an #if DEBUG statement

  4. Jocelyn Coulmance says:

    If we want to avoid boxing althogether, we should have methods "Lock" return a "TimedLock" instead of an "IDisposable".

    For C# language designers : it would be nice to have the "Dispose" method of a struct implementing "IDisposable" called automatically at the end of the block, without having to use "using". This would give us an exact equivalent of C++’s Resource Acquisition Is Initialization pattern.

  5. While we’re on the subject of boxing, why doesn’t foreach do the same optimization as using? foreach always seems to box the enumerator struct.

  6. That’s weird. As soon as I post this the same idea (and code) on my blog, someone comes up with it just a couple of days later. Great minds think alike? 🙂 I like the improvements though.

  7. David Bossert says:

    Why not just use a WaitHandle if you need a timeout? It still isn’t as fancy as a lock statement, but it’s a lot more palatable than that mess Ian posted. 🙂

  8. Mike Iles says:

    This is an interesting idea, but it means that you’re never going to be able to debug the cause of the deadlock! At least when a system deadlocks normally you can attach a debugger and figure out who’s holding the lock and fix the problem. With this approach all you get is an error message saying that an operation was aborted.

    It would be hard to include enough information at the point the lock timed out to debug the problem. The stack trace wouldn’t help. At the very least you would need to log the stack traces of all *other* threads in case one of them was holding the lock. (Is this possible?) Or each time a lock was acquired you would have to store the stack trace of the acquirer in some globally-visible spot (the overhead of this would be unacceptable). Or you would have to generate a dump of the process space and save it.

    (And we’re also leaving aside the problem of how to recover from such a serious error.)

  9. Mike,

    Although the stack trace won’t hold information about the other participant(s) of the deadlock, knowing one out of the crowd (more than likely, the crowd of two) significantly narrows down the problem. For example, increase holding time on the identified lock and try again. The next crash will give you another culprit. By varying holding time and with some deductive thinking, you can do figure out quite a bit of deadlock debugging.

  10. haacked says:

    Perhaps the TimedLock class could store its own stack trace after it acquires a lock in a static hashtable. (of course this would only be useful for a debug version of the class and would not be for production).

    For example something like this (a bit of pseudo coding):

    static HashTable _locks = new HashTable();

    public static IDisposable Lock (object o, TimeSpan timeout)

    {

    TimedLock tl = new TimedLock (o);

    if (!Monitor.TryEnter (o, timeout))

    {

    GetInfoFrom(_locks[o]);

    throw new LockTimeoutException(GetInfoFrom(_locks[o]););

    }

    _locks.Add(o, GetStackTrace());

    return tl;

    }

    public void Dispose ()

    {

    _locks.Remove(o);

    Monitor.Exit (target);

    }

    Could this work?

  11. damien morton says:

    or simply add a trylock (object x, int timeout) statement to the language…

    Of course, with whidbey, it can be done like this:

    TimedLock.TryLock(obj, timeout, delegate

    {

    statements…..

    });

    where trylock takes an anonymous method to wrap its locking semantics around.

  12. David – WaitHandle is considerably more expensive than Monitor, so that’s one reason not to use it. Waiting for a WaitHandle will typically involve an API call and a kernel mode transition. Monitors can be acquired without calling into the OS kernel in the no-contention case, so they are very much faster.

    And in any case, the whole point of this is to make the *call site* easier to read. The fact that it’s a bit of a mess internally isn’t really the point. It’s one of those bits of utility code that is there to make everything else better.

    Moreover, using a WaitHandle actually makes things worse – then you’d have to go *back* to using a finally block. (Or you’d have to implement something of equal ugliness to this in order to guarantee that the WaitHandle got releasedd.)

    Chance and Jocelyn – thanks for catching the errors introduced by going to a struct model – I’ve now fixed my code. (I think.)

  13. Brett says:

    or simply lock (object x, int timeout)

  14. Brett says:

    Reading this entry … and various related thread eventually lead me to the Reader/Writer lock, where I learned that unfortunately there is no built-in support via a lock-like statement ;-(

  15. Haacked says:

    I wrote an update I thought you might be interested. It is a DEBUG version where the LockTimeoutException contains a stack trace of the thread that is holding a lock to the object when acquiring the lock fails due to blocking.

    http://haacked.europe.webmatrixhosting.net/archive/2004/04/17/333.aspx

    I’d love to hear your thoughts and comments.