<html><body><div style="color:#000; background-color:#fff; font-family:times new roman, new york, times, serif;font-size:12pt"><div><span><br></span></div><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;">

On 19Sep2012 23:25, X Ryl wrote: |   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 <a target="_blank" href="http://localstatus.py/">LocalStatus.py</a>, 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)<br>| x2 = LocalStatusFolder(name, repo)<br>| # In Thread 1<br>| x1.save()<br>| # In Thread 2<br>| x2.save()<br>| <br>| is not protected (it's racy).<br>| <br>| I think the intend was to protect against all instance access to the<br>| local status folder, that is, the "savelock" must be a static member<br>| of the class.<br><br>Cameron Simpson wrote:<br>No, it looks like it is to prevent simultaneous access to a folder of a<br>particular name. Your solution (class variable) would prevent access to<br>_any_ LocalStatusFolder at the same time.<br><br>This code:<br><br>  x1 = LocalStatusFolder(name, repo)<br>  x2 = LocalStatusFolder(name, repo)<br><br>is the problem: if that _really_ occurs (create an object for the same<br>LocalStatusFolder more than once) then LocalStatusFolder needs to become<br>a factory function to return a previous instance if the name is reused.<br><br>.............................<br>Thanks to X and Cameron for

 finding this.<br><br>I agree with the upgrading to a Factory pattern here, replacing the current code which appears to be vulnerable to a race condition bug while OLI is running in multi threaded mode.  A race condition would possibly corrupt data in the LocalStatus, LocalStatusSQLite, and UIDMaps local databases/files.<br><br>While we're at it- why not upgrade UIDMaps to SQLite ?  Seems like it should be a higher quality solution than the flat file...<br><br>Chris<br><br> </div> </div>  </div></body></html>