| Key: |
POOL-102
|
| Type: |
Bug
|
| Status: |
Resolved
|
| Resolution: |
Fixed
|
| Priority: |
Minor
|
| Assignee: |
Unassigned
|
| Reporter: |
John Sumsion
|
| Votes: |
1
|
| Watchers: |
2
|
|
If you were logged in you would be able to see more operations.
|
|
|
| Resolution Date: |
22/Jun/07 06:13 AM
|
In the following GenericObjectPool snippet inside borrowObject(), InterruptedException is caught and ignored.
case WHEN_EXHAUSTED_BLOCK:
try {
if(_maxWait <= 0) {
wait();
} else {
wait(_maxWait);
}
} catch(InterruptedException e) {
// ignored
}
There are two problems here:
1) a thread waiting forever to get an object out of the pool will NEVER terminate, even if interrupted
2) even if you put a "throw e" in, it will still be wrong because the thread's interrupted status is not preserved
This will cause cancellation problems for threads that are inside borrowObject() that want to terminate early ONLY if they are interrupted.
For example, if a borrow-and-wait-forever was running on an pooled executor thread in Java 1.5 and the executor service tried to cancel a task and that task had early-termination logic in it that checked interrupted status to terminate early, the task would never be cancelled.
For us, this is minor because we are on a Tomcat request thread that has to wait for this resource to continue, but for others that have pools of stuff that are being used by time-bound tasks, it's inconvenient to write code that waits for what ends up being arbitrary time periods for a wait. It would be easier to just say wait forever and allow interruption by someone else who is watching me.
Suggestion: make the code read like this:
case WHEN_EXHAUSTED_BLOCK:
try {
if(_maxWait <= 0) { wait(); } } else {
wait(_maxWait);
}
} catch(InterruptedException e) {
Thread.currentThread().interrupt();
throw e;
}
|
|
Description
|
In the following GenericObjectPool snippet inside borrowObject(), InterruptedException is caught and ignored.
case WHEN_EXHAUSTED_BLOCK:
try {
if(_maxWait <= 0) {
wait();
} else {
wait(_maxWait);
}
} catch(InterruptedException e) {
// ignored
}
There are two problems here:
1) a thread waiting forever to get an object out of the pool will NEVER terminate, even if interrupted
2) even if you put a "throw e" in, it will still be wrong because the thread's interrupted status is not preserved
This will cause cancellation problems for threads that are inside borrowObject() that want to terminate early ONLY if they are interrupted.
For example, if a borrow-and-wait-forever was running on an pooled executor thread in Java 1.5 and the executor service tried to cancel a task and that task had early-termination logic in it that checked interrupted status to terminate early, the task would never be cancelled.
For us, this is minor because we are on a Tomcat request thread that has to wait for this resource to continue, but for others that have pools of stuff that are being used by time-bound tasks, it's inconvenient to write code that waits for what ends up being arbitrary time periods for a wait. It would be easier to just say wait forever and allow interruption by someone else who is watching me.
Suggestion: make the code read like this:
case WHEN_EXHAUSTED_BLOCK:
try {
if(_maxWait <= 0) { wait(); } } else {
wait(_maxWait);
}
} catch(InterruptedException e) {
Thread.currentThread().interrupt();
throw e;
} |
Show » |
|
Thread waiting forever for borrowObject() cannot be interrupted
