[
https://issues.apache.org/jira/browse/HTTPCLIENT-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12766560#action_12766560
]
Tim Boemker commented on HTTPCLIENT-881:
----------------------------------------
The current version of the code still has a race condition. In
AbstractClientConnection.java, it's possible for abortConnection and
releaseConnection both to read shutdown as false and for both to continue
execution. ThreadSafeClientConnManager.releaseConnection is not thread-safe
when two threads are trying to release the same connection.
I am following these steps to demonstrate the race condition:
1) Let abortConnection and releaseConnection both step through the line that
tests whether shutdown is already set.
2) Let the thread running releaseConnection run just past the line in
ThreadSafeClientConnManager.releaseConnection that sets local variable reusable
from hca.isMarkedReusable.
3) Let the thread running abortConnection run to the same point.
4) Let the thread running releaseConnection finish.
5) Let the thread running abortConnection finish.
6) Attempt another request to the same host.
These steps consistently produce the following error:
java.lang.IllegalStateException: There is no entry that could be dropped.
at
org.apache.http.impl.conn.tsccm.RouteSpecificPool.dropEntry(RouteSpecificPool.java:244)
at
org.apache.http.impl.conn.tsccm.ConnPoolByRoute.freeEntry(ConnPoolByRoute.java:418)
at
org.apache.http.impl.conn.tsccm.ThreadSafeClientConnManager.releaseConnection(ThreadSafeClientConnManager.java:254)
at
org.apache.http.impl.conn.AbstractClientConnAdapter.abortConnection(AbstractClientConnAdapter.java:338)
at
org.apache.http.impl.client.DefaultRequestDirector.abortConnection(DefaultRequestDirector.java:1058)
at
org.apache.http.impl.client.DefaultRequestDirector.execute(DefaultRequestDirector.java:566)
at
org.apache.http.impl.client.AbstractHttpClient.execute(AbstractHttpClient.java:674)
at
org.apache.http.impl.client.AbstractHttpClient.execute(AbstractHttpClient.java:609)
...
I propose the following change to the current version of
AbstractClientConnection.java:
Index: AbstractClientConnAdapter.java
===================================================================
--- AbstractClientConnAdapter.java (revision 825909)
+++ AbstractClientConnAdapter.java (working copy)
@@ -311,20 +311,24 @@
}
public void releaseConnection() {
- if (shutdown) {
- return;
+ synchronized(this) {
+ if (shutdown) {
+ return;
+ }
+ shutdown = true;
}
- shutdown = true;
if (connManager != null) {
connManager.releaseConnection(this, duration,
TimeUnit.MILLISECONDS);
}
}
public void abortConnection() {
- if (shutdown) {
- return;
+ synchronized(this) {
+ if (shutdown) {
+ return;
+ }
+ shutdown = true;
}
- shutdown = true;
unmarkReusable();
try {
shutdown();
With this code change, the regression tests pass and, I think, the problem is
fixed.
> AbstractClientConnAdapter doesn't ensure that only one of
> ConnectionReleaseTrigger.abortConnection, .releaseConnection has effect
> ---------------------------------------------------------------------------------------------------------------------------------
>
> Key: HTTPCLIENT-881
> URL: https://issues.apache.org/jira/browse/HTTPCLIENT-881
> Project: HttpComponents HttpClient
> Issue Type: Bug
> Components: HttpConn
> Affects Versions: 4.0 Final
> Reporter: Tim Boemker
> Fix For: 4.1 Alpha1
>
> Attachments: HTTPCLIENT-881.patch
>
>
> If HttpUriRequest.abort() is called at about the same time that the request
> completes, it's possible for an aborted connection to be returned to the
> pool. The next time the connection is used, HttpClient.execute fails without
> retrying, throwing this exception:
> java.io.IOException: Connection already shutdown
> at
> org.apache.http.impl.conn.DefaultClientConnection.opening(DefaultClientConnection.java:112)
> at
> org.apache.http.impl.conn.DefaultClientConnectionOperator.openConnection(DefaultClientConnectionOperator.java:120)
> at
> org.apache.http.impl.conn.AbstractPoolEntry.open(AbstractPoolEntry.java:147)
> at
> org.apache.http.impl.conn.AbstractPooledConnAdapter.open(AbstractPooledConnAdapter.java:101)
> at
> org.apache.http.impl.client.DefaultRequestDirector.execute(DefaultRequestDirector.java:381)
> at
> org.apache.http.impl.client.AbstractHttpClient.execute(AbstractHttpClient.java:641)
> at
> org.apache.http.impl.client.AbstractHttpClient.execute(AbstractHttpClient.java:576)
> Steps to reproduce:
> 1) Set a breakpoint in ThreadSafeClientConnManager.releaseConnection just
> after "reusable" is set (and found to be true).
> 2) Run to the breakpoint in releaseConnection.
> 3) Call HttpUriRequest.abort.
> 4) Let releaseConnection complete.
> When the connection is next used, the exception will be thrown.
> Snippet from ThreadSafeClientConnManager:
> public void releaseConnection(ManagedClientConnection conn, long
> validDuration, TimeUnit timeUnit) {
> ...
> boolean reusable = hca.isMarkedReusable();
> if (log.isDebugEnabled()) { //
> breakpoint here
> if (reusable) {
> log.debug("Released connection is reusable.");
> } else {
> log.debug("Released connection is not reusable.");
> }
> }
> hca.detach();
> if (entry != null) {
> connectionPool.freeEntry(entry, reusable, validDuration,
> timeUnit);
> }
> }
> }
> I think that AbstractClientConnAdapter should be modified as follows:
> 1) Add "released" flag:
> /** True if the connection has been released. */
> private boolean released;
> 2) Modify abortConnection:
> public void abortConnection() {
> synchronized(this) {
> if (aborted || released) {
> return;
> }
> aborted = true;
> }
> unmarkReusable(); // this line and all that follow unchanged
> 3) Modify releaseConnection:
> public void releaseConnection() {
> synchronized(this) {
> if (aborted || released) {
> return;
> }
> released = true;
> }
> if (connManager != null) {
> connManager.releaseConnection(this, duration,
> TimeUnit.MILLISECONDS);
> }
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
