-1

I want to reask my previous question how to obtain a lock in two places but release on one place? because it seems too old and noone sees my updated code.

The question is - is my code correct and how to fix it if not? I've tried similar code in the application and it hangs, but I don't now why, so I guess probably my code still wrong...

public void obtainLock() {
    if (needCallMonitorExit == false) {
        Monitor.Enter(lockObj);
        needCallMonitorExit = true;
    }
    // doStuff
}

public void obtainReleaseLock() {
    try {
        lock (lockObj) {
            // doAnotherStuff
        }
    } finally {
        if (needCallMonitorExit == true) {
            needCallMonitorExit = false;
            Monitor.Exit(lockObj);
        }
    }
}

One of my methods should obtain the lock. another method should obtain the same lock and release it. sometimes just obtainReleaseLock is called. sometimes obtainLock is called (probably several times) and after a while obtainReleaseLock is called. These two methods are always called from the same thread, however lockObj is used in another thread for synchronization.

Community
  • 1
  • 1
Oleg Vazhnev
  • 23,239
  • 54
  • 171
  • 305
  • Editing the previous question with your new code will bump it to the top of the list, just like asking a new question. Reasking a question is not viewed as an acceptable practice on StackOverflow. – BradleyDotNET Apr 05 '14 at 08:03

2 Answers2

0

If you really need to go this way and don't want to use the alternatives provided by Marc, at least, put it into its own class:

public class LockObject
{
    object _syncRoot = new object();
    object _internalSyncRoot = new object();

    public LockToken Lock()
    {
        lock(_internalSyncRoot)
        {
            if(!_hasLock)
            {
                Monitor.Enter(_syncRoot);
                _hasLock = true;
            }
            return new LockToken(this);
        }
    }

    public void Release()
    {
        lock(_internalSyncRoot)
        {
            if(!_hasLock)
                return;
            Monitor.Exit(_syncRoot);
            _hasLock = false;
        }
    }
}

public class LockToken : IDisposable
{
    LockObject _lockObject;
    public LockToken(LockObject lockObject) { _lockObject = lockObject; }

    public void Dispose() { _lockObject.Release(); }
}

This would allow you to use it like this:

LockObject _lockObj = new LockObject();

public void obtainLock()
{
    _lockObj.Lock();
    // doStuff
}

public void obtainReleaseLock()
{
    using(_lockObj.Lock())
    {
        // doAnotherStuff
    }
}

One word of caution:

If your thread is aborted after the call to obtainLock and before the call to obtainReleaseLock, your program will deadlock.

Community
  • 1
  • 1
Daniel Hilgarth
  • 171,043
  • 40
  • 335
  • 443
  • thanks for this code, i will try it. why .NET doesn't have somthing like that? I think it's very common to obtain a lock several times and then release in one place... – Oleg Vazhnev Sep 28 '11 at 08:07
  • @javapowered: Actually, it isn't that common. I never had to use something like that. – Daniel Hilgarth Sep 28 '11 at 08:46
0

If you use Monitor.Enter you will need to call exactly that many Monitor.Exit calls on the same object, on the same thread. By using a lock block this is automatically done for you - as soon as you are outside the scope of the lock block, the lock count is decreased. Within the same thread it is perfectly legal to lock multiple times on the same object (via both mechanisms).

Keeping that in mind, maintaining your own "lock state" variable would only make things more complex. What if two threads access the needCallMonitorExit variable at the same time?

If I were you I would stick to lock blocks, and re-order code to fit inside them. Put as little code inside the blocks as possible.

C.Evenhuis
  • 25,996
  • 2
  • 58
  • 72
  • needCallMonitorExit is accessed from one thready only... – Oleg Vazhnev Sep 28 '11 at 07:52
  • My bad, misread that in your question. The rest still stands though, I can't think of a reason why things should be this complex? – C.Evenhuis Sep 28 '11 at 08:18
  • hot to make what I want to make simpler? I need to obtain a lock in one method, but release in another, those I can not use `lock{}` statement – Oleg Vazhnev Sep 28 '11 at 08:38
  • How do you know you _need_ to obtain a lock and release it in a different method? The whole idea behind a lock is to perform a group of operations as an atomic operation. What are you _really_ trying to achieve here? – C.Evenhuis Sep 28 '11 at 13:15
  • trading software consists of two parts - ordersManager (which calculates stores and manages orders) and ordersExecutor (which only sends transaction to network). they both need "orders". if some order is updated (filled for example.) ordersExecutor should be blocked until orders recalculated. ordersRecalculation is separate method and when orders are recalculating ordersExecutor should be blocked as well.... I'm not sure if you will understand that, this is the schema invented myself :). two actual methods are `orderUpdated` and `recalculateOrders` – Oleg Vazhnev Sep 28 '11 at 14:35
  • `orderUpdated` should block `ordersExecutor` until `recalculateOrders` is called and finished. `recalculateOrders` itself should also block `ordersExecutor` – Oleg Vazhnev Sep 28 '11 at 14:36
  • So why not `lock (xxx) { orderUpdated(); recalculateOrders(); }` ? – C.Evenhuis Sep 29 '11 at 10:31
  • `orderUpdated` may be called several times before `recalculateOrders` called. several orders may be updated, i don't know exact number. Of course i can significantly change design to use `lock{}` (group everything in one method) but I don't want, I think current design is better – Oleg Vazhnev Sep 29 '11 at 11:21