Java Driver
  1. Java Driver
  2. JAVA-635

SimplePool.permitAcquired method swallows InterruptedException

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major - P3 Major - P3
    • Resolution: Fixed
    • Affects Version/s: 2.9.0
    • Fix Version/s: 2.9.1
    • Component/s: None
    • Labels:
      None
    • Environment:
      3.5.2-3.fc17.x86_64/JDK1.7.0_06 (x64)
    • Operating System:
      ALL
    • # Replies:
      6
    • Last comment by Customer:
      false

      Description

      SimplePool.permitAcquired(long) method catches InterruptedException but fails to restore interrupted state of the thread thus effectively suppressing the effect of thread interruption.
      Here is the (abbreviated) code in question (SimplePool.java lines 146-159):

          private boolean permitAcquired(final long waitTime) {
              try {
                  // ......
              } catch (InterruptedException e) {
                  return false;
              }
          }
      

      Instead it should be:

          private boolean permitAcquired(final long waitTime) {
              try {
                  // ......
              } catch (InterruptedException e) {
                  Thread.currentThread().interrupt();
                  return false;
              }
          }
      

        Activity

        Hide
        Jeff Yemin
        added a comment -

        Problem is there is no way to distinguish between a timeout and an interrupt. The choices are to set the interrupt flag, as suggested here, or throw a different runtime exception, like MongoInterruptedException. I prefer the second option.

        Show
        Jeff Yemin
        added a comment - Problem is there is no way to distinguish between a timeout and an interrupt. The choices are to set the interrupt flag, as suggested here, or throw a different runtime exception, like MongoInterruptedException. I prefer the second option.
        Hide
        Adam Warski
        added a comment -

        I'd also log a warning so that the exception that is thrown later (about timeout) isn't too surprising (once you see a timeout exception you don't normally check the interrupted status of the thread).

        See: https://groups.google.com/forum/?fromgroups=#!topic/mongodb-user/KjQc2h1kggM

        Show
        Adam Warski
        added a comment - I'd also log a warning so that the exception that is thrown later (about timeout) isn't too surprising (once you see a timeout exception you don't normally check the interrupted status of the thread). See: https://groups.google.com/forum/?fromgroups=#!topic/mongodb-user/KjQc2h1kggM
        Hide
        Alexei Krainiouk
        added a comment -

        I don't think you really need to distinguish between timeout and interupted conditions in your case. According to Semaphore.tryAcquire(long, TimeUnit) as well as Semaphore.acquire() contracts, they will only throw InterruptedException if the current thread was actually interrupted, on timeout tryAcquire will quietly return false and acquire() wait indefinitely. So, it seems to me that proposed fix should work just fine.

        Show
        Alexei Krainiouk
        added a comment - I don't think you really need to distinguish between timeout and interupted conditions in your case. According to Semaphore.tryAcquire(long, TimeUnit) as well as Semaphore.acquire() contracts, they will only throw InterruptedException if the current thread was actually interrupted, on timeout tryAcquire will quietly return false and acquire() wait indefinitely. So, it seems to me that proposed fix should work just fine.
        Hide
        auto (Inactive)
        added a comment -

        Author:

        {u'date': u'2012-09-04T13:16:54-07:00', u'name': u'Jeff Yemin', u'email': u'jeff.yemin@10gen.com'}

        Message: JAVA-635: Added MongoInterruptedException, a runtime exception that wraps an InterruptedException. An instance of this class is thrown in cases where the driver is sleeping or waiting on a condition, and has to catch InterruptedException and do something besides swallowing it. Since InterruptedException is checked and so can't be thrown from methods that don't declare it, best thing to do it throw a runtime exception that can be handled in the application
        Branch: master
        https://github.com/mongodb/mongo-java-driver/commit/2bbfac6c2e00cdc3968ba671a282bd3da2057d47

        Show
        auto (Inactive)
        added a comment - Author: {u'date': u'2012-09-04T13:16:54-07:00', u'name': u'Jeff Yemin', u'email': u'jeff.yemin@10gen.com'} Message: JAVA-635 : Added MongoInterruptedException, a runtime exception that wraps an InterruptedException. An instance of this class is thrown in cases where the driver is sleeping or waiting on a condition, and has to catch InterruptedException and do something besides swallowing it. Since InterruptedException is checked and so can't be thrown from methods that don't declare it, best thing to do it throw a runtime exception that can be handled in the application Branch: master https://github.com/mongodb/mongo-java-driver/commit/2bbfac6c2e00cdc3968ba671a282bd3da2057d47
        Hide
        auto (Inactive)
        added a comment -

        Author:

        {u'date': u'2012-09-07T10:49:06-07:00', u'name': u'Jeff Yemin', u'email': u'jeff.yemin@10gen.com'}

        Message: JAVA-635: Improved test coverage for InterruptedException/MongoInterruptedException
        Branch: master
        https://github.com/mongodb/mongo-java-driver/commit/79f979c82efcf40aed3072ed3269c96a1ef143b7

        Show
        auto (Inactive)
        added a comment - Author: {u'date': u'2012-09-07T10:49:06-07:00', u'name': u'Jeff Yemin', u'email': u'jeff.yemin@10gen.com'} Message: JAVA-635 : Improved test coverage for InterruptedException/MongoInterruptedException Branch: master https://github.com/mongodb/mongo-java-driver/commit/79f979c82efcf40aed3072ed3269c96a1ef143b7
        Hide
        Jeff Yemin
        added a comment -

        All methods which communicate with the server can now throw a MongoInterruptedException, a subclass of MongoException (which subclasses RuntimeException) which can be caught and handled in application code. The interrupt bit is not set.

        Show
        Jeff Yemin
        added a comment - All methods which communicate with the server can now throw a MongoInterruptedException, a subclass of MongoException (which subclasses RuntimeException) which can be caught and handled in application code. The interrupt bit is not set.

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              Days since reply:
              2 years, 25 weeks, 6 days ago
              Date of 1st Reply: