views:

33

answers:

2

I have a bunch of different methods that are not supposed to run concurrently, so I use a single lock to synchronize them. Looks something like this:

selected_method = choose_method()
with lock:
    selected_method()

In some of these methods, I sometimes call a helper function that does some slow network IO. (Let's call that one network_method()). I would like to release the lock while this function is running, to allow other threads to continue their processing.

One way to achieve this would be by calling lock.release() and lock.acquire() before and after calling the network method. However, I would prefer to keep the methods oblivious to the lock, since there are many of them and they change all the time.

I would much prefer to rewrite network_method() so that it checks to see whether the lock is held, and if so release it before starting and acquire it again at the end.

Note that network_method() sometimes gets called from other places, so it shouldn't release the lock if it's not on the thread that holds it.

I tried using the locked() method on the Lock object, but that method only tells me whether the lock is held, not if it is held by the current thread.

By the way, lock is a global object and I'm fine with that.

+1  A: 

I would much prefer to rewrite network_method() so that it checks to see whether the lock is held, and if so release it before starting and acquire it again at the end.

Note that network_method() sometimes gets called from other places, so it shouldn't release the lock if it's not on the thread that holds it.

This just sounds like entirely the wrong thing to do :(

For a start, it's bad to have a function that sometimes has some other magical side-effect depending on where you call it from. That's the sort of thing that is a nightmare to debug.

Secondly, a lock should have clear acquire and release semantics. If I look at code that says "lock(); do_something(); unlock();" then I expect it to be locked for the duration of do_something(). In fact, it is also telling me that do_something() requires a lock. If I find out that someone has written a particular do_something() which actually unlocks the lock that I just saw to be locked, I will either (a) fire them or (b) hunt them down with weapons, depending on whether I am in a position of seniority relative to them or not.

By the way, lock is a global object and I'm fine with that.

Incidentally, this is also why globals are bad. If I modify a value, call a function, and then modify a value again, I don't want that function in the middle being able to reach back out and modify this value in an unpredictable way.

My suggestion to you is this: your lock is in the wrong place, or doing the wrong thing, or both. You say these methods aren't supposed to run concurrently, but you actually want some of them to run concurrently. The fact that one of them is "slow" can't possibly make it acceptable to remove the lock - either you need the mutual exclusion during this type of operation for it to be correct, or you do not. If the slower operation is indeed inherently safe when the others are not, then maybe it doesn't need the lock - but that implies the lock should go inside each of the faster operations, not outside them. But all of this is dependent on what exactly the lock is for.

Kylotan
This is a great answer, in that it explains why there's probably no proper way to do this. I still have the original issue that made me create the lock, but it's something for a different question.
itsadok
A: 

Why not just do this?

with lock:
    before_network()
do_network_stuff()
with lock:
    after_network()
FogleBird