Robert Crocombe commented on ZOOKEEPER-645:

I can address point (1) in the original comment: having the session ID breaks 
the lock recipe.  It looks like the session ID was added per:


but the sorting via ZNodeName is worse than starvation: it causes clients with 
low session IDs not to see that clients with higher session IDs have acquired 
the lock which causes the function to return true: so you could theoretically 
have as many lock owners as clients as long as they acquire the lock in 
descending session ID order.

I fixed this in a local branch by replacing ZNodeName with a different class 
that ignores the session ID when sorting.  My test for WriteLockTest became:
        * Test that the bug which makes it possible to acquire a lock in two
        * different sessions simultaneously is fixed. Bug occurs because 
        * session ID in node name results in sorting in
        * LockZooKeeperOperation.execute() such that low session ID clients do 
        * see that higher session ID clients already have the lock.
       public void testSessionOrderingBugFix() throws Exception {
               // session IDs are presumably assigned in order, but perform 
some checks
               final ZooKeeper zooA = createClient();
               final ZooKeeper zooB = createClient();

               final ZooKeeper lowZoo = zooA.getSessionId() < 
zooB.getSessionId() ? zooA
                               : zooB;
               final ZooKeeper highZoo = zooA.getSessionId() < 
zooB.getSessionId() ? zooB
                               : zooA;

               final WriteLock highLock = new WriteLock(highZoo, dir, null);
               final WriteLock lowLock = new WriteLock(lowZoo, dir, null);

               boolean highLocked = highLock.lock();

               // If the bug is present, this attempt to lock will succeed.
               boolean lowLocked = lowLock.lock();


> Bug in WriteLock recipe implementation?
> ---------------------------------------
>                 Key: ZOOKEEPER-645
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-645
>             Project: Zookeeper
>          Issue Type: Bug
>          Components: recipes
>    Affects Versions: 3.2.2
>         Environment: 3.2.2 java 1.6.0_12
>            Reporter: Jaakko Laine
>            Assignee: Jaakko Laine
>            Priority: Minor
>             Fix For: 3.3.0
>         Attachments: 645-fix-findPrefixInChildren.patch
> Not sure, but there seem to be two issues in the example WriteLock:
> (1) ZNodeName is sorted according to session ID first, and then according to 
> znode sequence number. This might cause starvation as lower session IDs 
> always get priority. WriteLock is not thread-safe in the first place, so 
> having session ID involved in compare operation does not seem to make sense.
> (2) if findPrefixInChildren finds previous ID, it should add dir in front of 
> the ID

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

Reply via email to