Probably misunderstanding of the Threading/Lock code

[X Ryl]

Hi all,

  While I was getting similar erroneous behaviour as reported in the list
previously, I've tried to track the issue in the code and found a major
issue.
In LocalStatus.py, LocalStatusSQLite.py,UIDMaps.py,
I'm seeing this pattern:

class LocalStatusFolder(BaseFolder):
    def __init__(self, name, repository):
    [...]
        self.savelock = threading.Lock()

    def save(self):
        with self.savelock:

I wonder the initial intend, but as I understand it, each INSTANCE of
the class will have its own lock/mutex, so the acquiring of the lock
in the method will only prevent mutual access if the same instance is
used in multiple thread.
That means that such code:
x1 = LocalStatusFolder(name, repo)
x2 = LocalStatusFolder(name, repo)
# In Thread 1
x1.save()
# In Thread 2
x2.save()

is not protected (it's racy).

I think the intend was to protect against all instance access to the
local status folder, that is, the "savelock" must be a static member
of the class.
In Python, it should be declared and used like this:

class LocalStatusFolder(BaseFolder):
    savelock = threading.Lock() # Notice it's out of the init method
    def __init__(self, name, repository):
    [...]
        self.savelock = threading.Lock()

    def save(self):
        with LocalStatusFolder.savelock: # Notice the use of the class
name and not self

I've not tracked where the instance of the LocalStatusFolder are used,
but you probably know better than me, but I think the corruption of
the local status folder reported by some user are caused by this.
The same applies the other files as well.

Best regards,
X
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/offlineimap-project/attachments/20120919/cbd50fc6/attachment-0002.html>