Skip to content

Fixed NPE PoolingAsyncClientConnectionManager#82

Closed
carterkozak wants to merge 1 commit intoapache:masterfrom
carterkozak:async_conpool_npe
Closed

Fixed NPE PoolingAsyncClientConnectionManager#82
carterkozak wants to merge 1 commit intoapache:masterfrom
carterkozak:async_conpool_npe

Conversation

@carterkozak
Copy link
Copy Markdown
Contributor

@carterkozak carterkozak commented Aug 9, 2017

PoolingAsyncClientConnectionManager.validateAfterInactivity causes
an NPE when PoolEntries are initially created with no Connection.

Added logic to avoid checking the connection on every pool
checkout similar to the blocking pool.

Added an connection.isOpen check to http1.x connections leased
from the pool, although I'm unsure if it's at all effective.

NPE stack trace (happens on all requests when validateAfterInactivity is set):

Caused by: java.lang.NullPointerException
	at org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager$1.completed(PoolingAsyncClientConnectionManager.java:169)
	at org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager$1.completed(PoolingAsyncClientConnectionManager.java:152)
	at org.apache.hc.core5.concurrent.BasicFuture.completed(BasicFuture.java:122)
	at org.apache.hc.core5.pool.StrictConnPool.fireCallbacks(StrictConnPool.java:354)
	at org.apache.hc.core5.pool.StrictConnPool.lease(StrictConnPool.java:170)
	at org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager.lease(PoolingAsyncClientConnectionManager.java:151)
	at org.apache.hc.client5.http.impl.async.AsyncExecRuntimeImpl.acquireConnection(AsyncExecRuntimeImpl.java:87)
	at org.apache.hc.client5.http.impl.async.AsyncConnectExec.execute(AsyncConnectExec.java:150)
	at org.apache.hc.client5.http.impl.async.AsyncExecChainElement.execute(AsyncExecChainElement.java:54)
	at org.apache.hc.client5.http.impl.async.AsyncExecChainElement$1.proceed(AsyncExecChainElement.java:62)
	at org.apache.hc.client5.http.impl.async.AsyncProtocolExec.internalExecute(AsyncProtocolExec.java:170)
	at org.apache.hc.client5.http.impl.async.AsyncProtocolExec.execute(AsyncProtocolExec.java:133)
	at org.apache.hc.client5.http.impl.async.AsyncExecChainElement.execute(AsyncExecChainElement.java:54)
	at org.apache.hc.client5.http.impl.async.AsyncExecChainElement$1.proceed(AsyncExecChainElement.java:62)
	at org.apache.hc.client5.http.impl.async.AsyncRetryExec.internalExecute(AsyncRetryExec.java:67)
	at org.apache.hc.client5.http.impl.async.AsyncRetryExec.execute(AsyncRetryExec.java:123)
	at org.apache.hc.client5.http.impl.async.AsyncExecChainElement.execute(AsyncExecChainElement.java:54)
	at org.apache.hc.client5.http.impl.async.AsyncExecChainElement$1.proceed(AsyncExecChainElement.java:62)
	at org.apache.hc.client5.http.impl.async.AsyncRedirectExec.internalExecute(AsyncRedirectExec.java:93)
	at org.apache.hc.client5.http.impl.async.AsyncRedirectExec.execute(AsyncRedirectExec.java:210)
	at org.apache.hc.client5.http.impl.async.AsyncExecChainElement.execute(AsyncExecChainElement.java:54)
	at org.apache.hc.client5.http.impl.async.InternalHttpAsyncClient.executeChain(InternalHttpAsyncClient.java:161)
	at org.apache.hc.client5.http.impl.async.InternalHttpAsyncClient.access$400(InternalHttpAsyncClient.java:67)
	at org.apache.hc.client5.http.impl.async.InternalHttpAsyncClient$3.sendRequest(InternalHttpAsyncClient.java:251)
	at org.apache.hc.core5.http.nio.support.BasicClientExchangeHandler.produceRequest(BasicClientExchangeHandler.java:73)
	at org.apache.hc.client5.http.impl.async.InternalHttpAsyncClient.execute(InternalHttpAsyncClient.java:240)
	at org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient.execute(CloseableHttpAsyncClient.java:77)

PoolingAsyncClientConnectionManager.validateAfterInactivity causes
an NPE when PoolEntries are initially created with no Connection.

Added logic to avoid checking the connection on every pool
checkout similar to the blocking pool.

Added an connection.isOpen check to http1.x connections leased
from the pool, although I'm unsure if it's at all effective.

})));
} else {
if (!connection.isOpen()) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be half the implementation of the blocking connection isStale() check

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stale connection check (as an attempt to read from the socket) is only applicable to blocking connections.

asfgit pushed a commit that referenced this pull request Aug 9, 2017
PoolingAsyncClientConnectionManager.validateAfterInactivity causes
an NPE when PoolEntries are initially created with no Connection.

Added logic to avoid checking the connection on every pool
checkout similar to the blocking pool.

Added an connection.isOpen check to http1.x connections leased
from the pool, although I'm unsure if it's at all effective.

Closes PR #82
@ok2c
Copy link
Copy Markdown
Member

ok2c commented Aug 9, 2017

Please review and close.

@carterkozak carterkozak closed this Aug 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants