Probably misunderstanding of the Threading/Lock code

[chris coleman]

X wrote:


Well, I took a bit of time to find out why it does get corrupted.
There is already a kind of "factory" for making such LocalStatus instance (look in repository/LocalStatus.py).
This factory is not protected by any lock, so the code like this:
    def getfolder(self, foldername):
        """Return the Folder() object for a foldername"""
        if foldername in self._folders:
            return self._folders[foldername]

        folder = self.LocalStatusFolderClass(foldername, self)
        self._folders[foldername] = folder
        return folder

Can get corrupted if two threads call the method at the same time (both thread can test for existence of the foldername inside the _folders member, and both can create a local instance of LocalStatusFolder for it, provided any thread is interrupted/scheduled after the dictionnay lookup).
This should not happen often, but it can happen.

Also, this part is even more wrong:
    def makefolder(self, foldername):
[...]
        # Invalidate the cache.
        self._folders = {}

If any thread call makefolder, then the next getfolder call will re-create the LocalStatusFolder instance, and as such, will completely corrupt the filesystem.

I'm continuing to dig inside the code to understand where/when the thread are created and used, but I think, as a temporary fix, I would put the LocalStatusFolder's Lock as a class instance so all threads will be serialized when touching the filesystem, thus preventing Maildir corruption like it's the case currently.

Then, when I get more understanding of the code, I can probably help fixing the root of the issue, instead of the effect.
Do you have a kind of "2mn tour inside the inner working of OFI" document somewhere ?

Cheers,
X

Hi X,

First thanks again for digging in even deeper and finding that bug!

The closest thing to "2mn tour inside the inner workings of OLI" (I like to call it OLI for OffLineImap) is this page:
http://docs.offlineimap.org/en/latest/API.html

It's generated from the comments in the code, so it's only as detailed as the comments in the code - it doesn't explain how the algorithms are supposed to work or the big picture of the sync algorithm or the way the threading or the idling are supposed to work.

Should be enough to get a grip on the "external parts of the internals" and start hacking.

It covers 7 methods 4 classes and the meanings of the exceptions.

And it goes without saying, improvements in this documentation will certainly allow coders to more easily improve OLI's code quality/reliability.

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/offlineimap-project/attachments/20120924/78fa9e99/attachment-0003.html>