Probably misunderstanding of the Threading/Lock code

[Cameron Simpson]

On 19Sep2012 23:25, X Ryl <boite.pour.spam at gmail.com> 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 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.

No, it looks like it is to prevent simultaneous access to a folder of a
particular name. Your solution (class variable) would prevent access to
_any_ LocalStatusFolder at the same time.

This code:

  x1 = LocalStatusFolder(name, repo)
  x2 = LocalStatusFolder(name, repo)

is the problem: if that _really_ occurs (create an object for the same
LocalStatusFolder more than once) then LocalStatusFolder needs to become
a factory function to return a previous instance if the name is reused.

Cheers,
-- 
Cameron Simpson <cs at zip.com.au>

Yes, we plot no less than the destruction of the West. Just the other day a
friend and I came up with the most pernicious academic scheme to date for
toppling the West: He will kneel behind the West on all fours. I will push it
backwards over him.     - Michael Berube