Hello,
Perhaps it's low priority since the comments of
BasicClientConnectionManager.java does say it ought to be handled by only
one thread but I would like to point out that it is not thread safe with
respect to shutdown in case you're interested.
The attached file includes a test I added to
httpclient/src/test/java/org/apache/http/impl/conn/TestBasicConnManager.java
that will demonstrate a null pointer exception raised by potentially
improper synchronization in BasicClientConnectionManager in relatively
short order.
Problem:
Since the assignment to variable shutdown in the shutdown method (line 262)
is not part of the synchronize block and none of the assertNotShutdown
method calls are within the synchronized blocks of their enclosing methods,
it is possible to have threads execute the commands of
BasicClientConnectionManager methods in the following sequence.
Thread 1
releaseConnection (or some other method that uses assertNotShutdown is
called)
assertNotShutdown - Passes - Line 183
Thread 2
shutdown
the shutdown flag becomes true - Line 262
shutdown releases this.poolEntry and this.conn in the synchronized(this)
block
Thread 1
release connection's synchronized(this) block get's executed
this.poolEntry is null in the try block causing a null pointer exception
- Line 211
this.poolEntry is null in the finally block causing a null pointer
exception - Line 224
Attached is a test case that will generate the attached stack-trace when
added to TestBasicConnManager and a possible fix (though the fix has a cost
of increased synchronization).
Thank you for your time.
Jonathan
-------------------------------------------------------------------------------
Test set: org.apache.http.impl.conn.TestBasicConnManager
-------------------------------------------------------------------------------
Tests run: 5, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.324 sec <<<
FAILURE!
testPrematureShutdown(org.apache.http.impl.conn.TestBasicConnManager) Time
elapsed: 1.014 sec <<< ERROR!
java.lang.RuntimeException: Iteration: 16
at
org.apache.http.impl.conn.TestBasicConnManager.testPrematureShutdown(TestBasicConnManager.java:268)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:601)
at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
at
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
at
org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:31)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:263)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:69)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:48)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)
at org.junit.runners.ParentRunner.run(ParentRunner.java:292)
at
org.apache.maven.surefire.junit4.JUnit4TestSet.execute(JUnit4TestSet.java:53)
at
org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:123)
at
org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:104)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:601)
at
org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:164)
at
org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:110)
at
org.apache.maven.surefire.booter.SurefireStarter.invokeProvider(SurefireStarter.java:172)
at
org.apache.maven.surefire.booter.SurefireStarter.runSuitesInProcessWhenForked(SurefireStarter.java:104)
at
org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:70)
Caused by: java.lang.NullPointerException
at
org.apache.http.impl.conn.BasicClientConnectionManager.releaseConnection(BasicClientConnectionManager.java:224)
at
org.apache.http.impl.conn.TestBasicConnManager.testPrematureShutdown(TestBasicConnManager.java:265)
... 31 more
@Test
public void testPrematureShutdown() throws InterruptedException {
//The error doesn't always happen right away since the order of operations
can vary based on thread scheduling
//but it happens for me by iteration 25 in most cases
for(int i = 0; i < 100000; i++) {
final BasicClientConnectionManager mgr = createConnManager(null);
final HttpHost target = getServerHttp();
final HttpRoute route = new HttpRoute(target, null, false);
final ManagedClientConnection conn = mgr.getConnection(route, null);
mgr.releaseConnection(conn, 100, TimeUnit.MILLISECONDS);
final class ShutdownRunnable implements Runnable {
public void run() {
try {
mgr.shutdown();
} catch (final Exception e) {
throw new RuntimeException(e);
}
}
}
//The problem is more reproducible if we slow down the threads below
that synchronize on mgr
//The problem should occur when shutdown and releaseConnection (or any
other method guarded by assertNotShutdown)
//is called
final class LockTheManager implements Runnable {
public void run() {
try {
synchronized(mgr) {
Thread.sleep(1000); //just to slow our synchronized
methods
}
} catch(Exception exc) {
}
}
}
try {
ManagedClientConnection connReq = mgr.getConnection(route, null);
final Thread shutdown = new Thread(new ShutdownRunnable());
final Thread lockout = new Thread(new LockTheManager());
lockout.start();
shutdown.start();
mgr.releaseConnection(connReq, 1000, TimeUnit.MILLISECONDS);
} catch(Exception iexc) {
if(iexc.getMessage() == null ||
!iexc.getMessage().equals("Connection manager has been shut down")) {
throw new RuntimeException("Failed on iteration: " + i, iexc);
}
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
