thread Locking/unlocking in constructor/destructor in python

Question

I have a class that is only ever accessed externally through static methods. Those static methods then create an object of the class to use within the method, then they return and the object is presumably destroyed. The class is a getter/setter for a couple config files and now I need to place thread locks on the access to the config files.

Since I have several different static methods that all need read/write access to the config files that all create objects in the scope of the method, I was thinking of having my lock acquires done inside of the object constructor, and then releasing in the destructor.

My coworker expressed concern that it seems like that could potentially leave the class locked forever if something happened. And he also mentioned something about how the destructor in python was called in regards to the garbage collector, but we're both relatively new to python so that's an unknown.

Is this a reasonable solution or should I just lock/unlock in each of the methods themselves?

Class A():
    rateLock = threading.RLock()
    chargeLock = threading.RLock()

    @staticmethod
    def doZStuff():
        a = A()
        a.doStuff('Z')

    @staticmethod
    def doYStuff():
        a = A()
        a.doStuff('Y')

    @synchronized(lock)
    def doStuff(self, type):
        if type == 'Z':
            otherstuff()
        elif type == 'B':
            evenmorestuff()

Is it even possible to get it to work that way with the decorator on doStuff() instead of doZStuff()

Update

Thanks for the answers everyone. The problem I'm facing is mostly due to the fact that it doesn't really make sense to access my module asynchronously, but this is just part of an API. And the team accessing our stuff through the API was complaining about concurrency issues. So I don't need the perfect solution, I'm just trying to make it so they can't crash our side or get garbage data back

Answer 1

You are right with the garbage collection, so it is not a good idea. Look into decorators, for writing synchronized functions.

Example: http://code.activestate.com/recipes/465057-basic-synchronization-decorator/

edit I'm still not 100% sure what you have in mind, so my suggestion may be wrong:

class A():
    lockZ = threading.RLock()
    lockY = threading.RLock()

    @staticmethod
    @synchroized(lockZ)
    def doZStuff():
        a = A()
        a.doStuff('Z')

    @staticmethod
    @synchroized(lockY)
    def doYStuff():
        a = A()
        a.doStuff('Y')

    def doStuff(self, type):
        if type == 'Z':
            otherstuff()
        elif type == 'B':
            evenmorestuff()

Answer 2

I don't know which platform you are on, but if you need to lock a file, well, you should probably use flock() if it is available instead of rolling your own locking routines.

Since you've mentioned that you are new to python, I must say that most of the time threads are not the solution in python. If your activity is CPU-bound, you should consider using multiprocessing. There is no concurrent execution because of GIL, remember? (this is true for most cases). If your activity is I/O bound, which I guess is the case, you should, perhaps, consider using an event-driven framework, like Twisted. That way you won't have to worry about deadlocks at all, I promise :)

Answer 3

Class A():
    rateLock = threading.RLock()
    chargeLock = threading.RLock()

    def doStuff(self,ratefile,chargefile):
        with A.rateLock:
            with open(ratefile) as f:
                # ...
        with A.chargeLock:
            with open(chargefile) as f:
                # ...

Using the with statement will guarantee that the (R)Lock is acquired and released in pairs. The release will be called even if there an exception occurs within the with-block.

You might also want to think about placing your locks around the file access block with open(...) as ... as tightly as you can so that the locks are not held longer than necessary.

Finally, the creation and garbage collection of a=A() will not affect the locks if (as above) the locks are class attributes (as opposed to instance attributes). The class attributes live in A.__dict__, rather than a.__dict__. So the locks will not be garbage collected until A itself is garbage collected.

Answer 4

Releasing locks in the destruction of objects is risky as has already been mentioned because of the garbage collector, because deciding when to call the __del__() method on objects is exclusively decided by the GC (usually when the refcount reaches zero) but in some cases, if you have circular references, it might never be called, even when the program exits.

If you are treating one specific configfile inside a class instance, then you might put a lock object from the Threading module inside it. Some example code of this:

from threading import Lock

class ConfigFile:
  def __init__(file):
    self.file = file
    self.lock = Lock()

  def write(self, data):
    self.lock.aquire()    
    <do stuff with file>
    self.lock.release()

# Function that uses ConfigFile object

def staticmethod():
    config = ConfigFile('myconfig.conf')
    config.write('some data')

You can also use locks in a With statement, like:

def write(self, data):
  with self.lock:
    <do stuff with file>

And Python will aquire and release the lock for you, even in case of errors that happens while doing stuff with the file.

Answer 5

However, if you HAVE TO acquire and release locks in constructors and destructors, then you really, really, really should give your design another chance. You should change your basic assumptions.

In any application: a "LOCK" should always be held for a short time only - as short as possible. That means - in probably 90% of all cases, you will acquire the lock in the same method that will also release the lock.

There should hardly be NEVER EVER a reason to lock/unlock an object in a RAII style. This is not what it was meant to become ;)

Let me give you an ekxample: you manage some ressources, those res. can be read from many threads at once but only one thread can write to them.

In a "naive" implementation you would have one lock per object, and whenever someone wants to write to it, then you will LOCK it. When multiple threads want to write to it, then you have it synchronyzed fairly, all safe and well, BUT: When thread says "WRITE", then we will stall, until the other threads decide to release the lock.

But please understand that locks, mutex - all these primitives were created to synchronize only a few lines of your source code. So, instead of making the lock part of you writeable object, you have only a lock for the very short time where it really is required. You have to invest more time and thoughts in your interfaces. But, LOCKS/MUTEXES were never meant to be "held" for more than a few microseconds.