<html><body><div style="color:#000; background-color:#fff; font-family:times new roman, new york, times, serif;font-size:12pt"><div><br><span></span></div><div style="color: rgb(0, 0, 0); font-size: 16px; font-family: times new roman,new york,times,serif; background-color: transparent; font-style: normal;"><br><span></span></div><div style="color: rgb(0, 0, 0); font-size: 16px; font-family: times new roman,new york,times,serif; background-color: transparent; font-style: normal;"><span><br></span></div>X wrote:<br><br>
<div style="font-family: times new roman, new york, times, serif; font-size: 12pt;"><div style="font-family: times new roman, new york, times, serif; font-size: 12pt;"><div id="yiv806272798">Well, I took a bit of time to find out why it does get corrupted.<div>There is already a kind of "factory" for making such LocalStatus instance (look in repository/LocalStatus.py).</div><div>This factory is not protected by any lock, so the code like this:</div>
<div><div><div> def getfolder(self, foldername):</div><div> """Return the Folder() object for a foldername"""</div><div> if foldername in self._folders:</div><div> return self._folders[foldername]</div>
<div><br></div><div> folder = self.LocalStatusFolderClass(foldername, self)</div><div> self._folders[foldername] = folder</div><div> return folder</div></div><div><br></div><div>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).</div>
<div>This should not happen often, but it can happen.</div><div><br></div><div>Also, this part is even more wrong:</div><div> def makefolder(self, foldername):</div><div>[...]</div><div> # Invalidate the cache.</div>
<div> self._folders = {}</div></div><div><br></div><div>If any thread call makefolder, then the next getfolder call will re-create the LocalStatusFolder instance, and as such, will completely corrupt the filesystem.</div>
<div><br></div><div>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.</div>
<div><br></div><div>Then, when I get more understanding of the code, I can probably help fixing the root of the issue, instead of the effect.</div><div>Do you have a kind of "2mn tour inside the inner working of OFI" document somewhere ?</div>
<div><br></div><div>Cheers,</div><div>X</div><div><br></div><div>Hi X,<br><br>First thanks again for digging in even deeper and finding that bug!<br><br>The closest thing to "2mn tour inside the inner workings of OLI" (I like to call it OLI for OffLineImap) is this page:<br>http://docs.offlineimap.org/en/latest/API.html<br><br>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.<br><br>Should be enough to get a grip on the "external parts of the internals" and start hacking.<br><br>It covers 7 methods 4 classes and the meanings of the exceptions.<br><br>And it goes without saying, improvements in this documentation will certainly allow coders to more easily improve OLI's code quality/reliability.<br><br>-Chris<br><br></div></div></div> </div>
</div></body></html>