2

In my unsafe class below, what can be done to prevent someone from executing the unsafe method without first obtaining the lock?

class Unsafe
{
    private static readonly object lockObj;

    void MethodA()
    {
        lock (lockObj)
        {
            // do some things
            DoUnsafeThing();
        }
    }

    void MethodB()
    {
        lock (lockObj)
        {
            // do some things
            DoUnsafeThing();
        }
    }

    void DoUnsafeThing()
    {
        if (callerHasLock)
            // Do the unsafe thing
        else
            return; // or throw some exception
    }
}

Obtaining the lock again inside DoUnsafeThing() is an option:

void DoUnsafeThing()
{
    lock (lockObj)
    {
        // Do the unsafe thing
    }
}

But DoUnsafeThing() can now be called by threads that don't already possess the lock.

NiloCK
  • 571
  • 1
  • 11
  • 33
  • As it is currently written, this would allow only a single thread to perform the `DoUnsafeThing()` method at a time. I'd probably move the lock to within that method, but otherwise looks fine. It you want it to be locked per thread, put `[ThreadStatic]` on the lock object. – Haney May 08 '14 at 17:32
  • I don't understand why you wouldn't put the lock in `DoUnsafeThing`, and only in `DoUnsafeThing`. There are other types of things you can use instead of lock (like `Monitor.TryEnter`) that would fail rather than wait if there is already a lock on it. – hatchet - done with SOverflow May 08 '14 at 17:39
  • 1
    @DavidHaney - why make the lock object ThreadStatic? Doesn't that defeat the whole purpose of locking? – hatchet - done with SOverflow May 08 '14 at 17:45
  • @hatchet: `MethodA` and `MethodB` are mutually unsafe throughout, and they contain an amount of duplicate code which I'd like to factor out into `DoUnsafeThing`. In hindsight, this was not clear in my question. – NiloCK May 08 '14 at 17:46
  • @hatchet yeah, in hindsight that's moronic. – Haney May 08 '14 at 17:50

1 Answers1

5

You should be able to use Monitor.IsEntered() to verify that the thread itself has already obtained the lock:

void DoUnsafeThing()
{
    if (Monitor.IsEntered(lockObj))
        // Do the unsafe thing
    else
        return; // or throw some exception
}

You can use the Monitor class to work with locks. In C#, the lock(...) statement is just syntactic sugar for Monitor.Enter(o); try {...} finally {Monitor.Exit(o);}. There are other options within it for fine-tuning. Remember, multi-threading is Hard. Know your toolset.

EDIT: (in response to framework version question update)

Prior to .NET 4.5, AFAIK the only way to handle this would be to use a thread-static Boolean alongside the synchronization object, set to true just after entering and false just before exiting. That same Boolean--call it callerHasLock, to conform with your code above--can then be tested within the lock context with the same result as Monitor.IsEntered.

Marc L.
  • 3,296
  • 1
  • 32
  • 42
  • I've voted this up (thanks!), but I'm going to hold off on marking it correct until I've translated a solution available in C# 3.5. – NiloCK May 08 '14 at 17:49
  • @NiloCK `Monitor` has been around since .NET 1.1 – Haney May 08 '14 at 17:52
  • 2
    You could do something similar to what Marc has suggested, but use Monitor.TryEnter which is in 3.5. A thread should be able to lock the same object twice. – hatchet - done with SOverflow May 08 '14 at 17:52
  • Ahh, the *method* is missing in 3.5 - I'm really off my ball on this question... ;) – Haney May 08 '14 at 17:53
  • @DavidHaney: The particular method `IsEntered` is new. @hatchet: That ought to do the trick. – NiloCK May 08 '14 at 17:54
  • @hatchet, I'm thinking that may not work, since like `Enter`, `TryEnter` communicates whether _any_ thread holds a lock on the lock-object--if not, it returns `true` whether the monitor is just- or already-entered. – Marc L. May 08 '14 at 18:00
  • @NiloCK - see this similar question http://stackoverflow.com/questions/391913/re-entrant-locks-in-c-sharp Just using plain `lock` in the three methods may work fine if waiting is fine, and `TryEnter` if you want failure if lock not already obtained. – hatchet - done with SOverflow May 08 '14 at 18:00
  • @NiloCK, please update your question and tags to reflect that dependency. – Marc L. May 08 '14 at 18:02
  • @hatchet, the problem is that neither `Enter` nor `TryEnter` will preclude (or help to preclude) an otherwise un-blocked thread from entering the meat of `DoUnsafeThing()` _if it had not previously obtained the lock_, which is precisely what is desired here. – Marc L. May 08 '14 at 18:16