JGroups
  1. JGroups
  2. JGRP-1277

Usage of TimeUnit class is inverted, causing incorrect conversions between time units

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved (View Workflow)
    • Priority: Minor Minor
    • Resolution: Done
    • Affects Version/s: 2.12
    • Fix Version/s: 2.12
    • Labels:
      None
    • Steps to Reproduce:
      Hide

      Taking the example code from the blog post and changing 2000 MILLLISECONDS to 2 SECONDS will show the problem. The 2 SECONDS will instead be converted to 0 which causing an immediate return of tryLock with a false.

      // lock.xml has to have a locking protocol in it
      JChannel ch=new JChannel("/home/bela/lock.xml");
      LockService lock_service=new LockService(ch);
      Lock lock=lock_service.getLock("mylock");
      if(lock.tryLock(2, TimeUnit.SECONDS)) {
          try {
              // access the resource protected by "mylock"
          }
          finally {
              lock.unlock();
          }
      }
      
      Show
      Taking the example code from the blog post and changing 2000 MILLLISECONDS to 2 SECONDS will show the problem. The 2 SECONDS will instead be converted to 0 which causing an immediate return of tryLock with a false. // lock.xml has to have a locking protocol in it JChannel ch= new JChannel( "/home/bela/lock.xml" ); LockService lock_service= new LockService(ch); Lock lock=lock_service.getLock( "mylock" ); if (lock.tryLock(2, TimeUnit.SECONDS)) { try { // access the resource protected by "mylock" } finally { lock.unlock(); } }
    • Similar Issues:
      Show 10 results 

      Description

      I have tried posting something to the blog at http://belaban.blogspot.com/2011/01/new-distributed-locking-service-in.html but it hasn't put up my comment yet, so I decided to create a JIRA as well.

      I have taken the latest 2.12 build of Beta1 which includes the new LockService class. This class is so much of an improvement how the DistributedLockManager was; great work!

      But anyways I found that when you try to acquire a lock with a wait, it is not converting the timeunits correctly unless it is in MILLISECONDS. This cannot be reproduced with the demo class since it always uses MILLISECONDS.

      The code in question is found on the Locking$ClientLock inner class.

              public boolean tryLock(long time, TimeUnit unit) throws InterruptedException {
                  return acquireTryLock(unit.convert(time, TimeUnit.MILLISECONDS), true);
              }
      

      The way this is, the code will convert the time value passed in as MILLISECONDS to the passed in TimeUnit. This simply needs to change to make the TimeUnit.MILLISECONDS the object of which the convert method is being called on and having unit as the second argument.

              public boolean tryLock(long time, TimeUnit unit) throws InterruptedException {
                  return acquireTryLock(TimeUnit.MILLISECONDS.convert(time, unit), true);
              }
      

      This obviously has a simple workaround of always passing in a TimeUnit of MILLISECONDS and having the time value already adjusted.

      And again this new feature looks to be very promising.

      Also when just checking the jgroups code base there are 3 other references to convert, 2 of which are also broken in this fashion.

      HashedTimingWheel.schedule
      TimeScheduler2.schedule

      Both of these references in the jgroup code base are always passed MILLISECONDS, so it would only be seen if client code were to interact with them somehow.

        Activity

        Hide
        Bela Ban
        added a comment -

        Thanks for finding this ! Note to self, the use of TimeUnit probably needs to be checked in a (new) 2.11.2 as well

        Show
        Bela Ban
        added a comment - Thanks for finding this ! Note to self, the use of TimeUnit probably needs to be checked in a (new) 2.11.2 as well
        Hide
        Bela Ban
        added a comment -

        Fixed. No need to fix on the 2.11 branch as LockService doesn't exist there and TimeScheduler.execute() is only called by internal code

        Show
        Bela Ban
        added a comment - Fixed. No need to fix on the 2.11 branch as LockService doesn't exist there and TimeScheduler.execute() is only called by internal code

          People

          • Assignee:
            Bela Ban
            Reporter:
            William Burns
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: