[JAVA-635] SimplePool.permitAcquired method swallows InterruptedException Created: 30/Aug/12  Updated: 10/Sep/12  Resolved: 07/Sep/12

Status: Closed
Project: Java Driver
Component/s: None
Affects Version/s: 2.9.0
Fix Version/s: 2.9.1

Type: Bug Priority: Major - P3
Reporter: Alexei Krainiouk Assignee: Jeffrey Yemin
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

3.5.2-3.fc17.x86_64/JDK1.7.0_06 (x64)



 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;
        }
    }



 Comments   
Comment by Jeffrey Yemin [ 07/Sep/12 ]

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.

Comment by auto [ 07/Sep/12 ]

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

Comment by auto [ 07/Sep/12 ]

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

Comment by Alexei Krainiouk [ 04/Sep/12 ]

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.

Comment by adamw [ 04/Sep/12 ]

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

Comment by Jeffrey Yemin [ 02/Sep/12 ]

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.

Generated at Thu Feb 08 08:52:44 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.