Skip to content

Make a best effort rebuild Apache client/pool after java.lang.Error - #7395

Draft
alextwoods wants to merge 3 commits into
masterfrom
alexwoo/apache_panic
Draft

alextwoods wants to merge 3 commits into
masterfrom
alexwoo/apache_panic

Conversation

@alextwoods

@alextwoods alextwoods commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Apache deliberately shuts its connection manager down when a java.lang.Error escapes a request (HTTPCLIENT-1924), which left the SDK's Apache clients permanently unusable. An OutOfMemoryError raised by unrelated application code while a request thread was inside the HTTP client took the shared pool down for the whole process: the application recovered, but every later SDK call failed with IllegalStateException: Connection pool shut down until the JVM was restarted. Apache 5.x behaves identically, so migrating to apache5-client did not help.

RecreatingHttpClientConnectionManager now wraps the pool in both modules. It honours Apache's intent by discarding the affected pool rather than resurrecting it, then builds a replacement on the next request. Throwing the pool away also disposes of the connection leases Apache's Error path never released, so the replacement starts at full capacity. An intentional close() goes through closePermanently() instead, so closing a client stays permanent.

Connections leased from a replaced pool are safe to release into the replacement: connect and routeComplete are entry-local, and AbstractConnPool.release (StrictConnPool.release in 5.x) is guarded by leased.remove(entry) and ignores an entry it never leased.

Pool metrics now report through the wrapper so they follow the current pool. Apache 5.x declares closeIdle on the concrete pooling manager rather than the interface, so IdleConnectionReaper reaps through a new IdleConnectionCloser interface, letting it hold one stable registration for the client's lifetime even when the pool underneath is replaced.

Verified with a repro harness that drives a real OutOfMemoryError through Apache's handler: previously 6/13 (apache) and 2/4 (apache5) runs left a dead pool, now 0, with no Connection pool shut down occurrences and leases correctly returned.

Summary

When a java.lang.Error such as an OutOfMemoryError escapes a request, Apache's MainClientExec catches it and destroys the shared connection manager (connManager.shutdown() in 4.x, close(CloseMode.IMMEDIATE) in 5.x) before rethrowing — which used to leave the SDK's client permanently dead, failing every later call with IllegalStateException: Connection pool shut down even after the application itself recovered. ApacheHttpClient/Apache5HttpClient now hand Apache a RecreatingHttpClientConnectionManager (one per module, in internal.conn) instead of the PoolingHttpClientConnectionManager directly, built from a supplier so it can make more pools. It treats Apache's shutdown as "discard and rebuild later": it honors the destruction (post-Error connection state is untrustworthy, and throwing the pool away also disposes of the leases Apache's Error path never released), flips to a NEEDS_RECREATE state, and lazily builds a fresh pool on the next lease — so an idle client is never resurrected, and metrics and the IdleConnectionReaper follow the current pool without rebuilding one. An intentional close() routes through a separate closePermanently() into a terminal CLOSED state, which is what keeps closing a client permanent even though Apache and the SDK otherwise call the same method. Two edges matter: a request that read the old pool moments before it died retries once on the replacement (the IllegalStateException Apache raises there is not retryable by the SDK), and in 5.x a connection returned to a replaced pool is rejected rather than silently ignored as in 4.x, so that specific exception is swallowed once a rebuild has happened. Requests genuinely in flight when the Error lands still fail, but with retryable IOExceptions rather than a permanently broken client.

Motivation and Context

Modifications

Testing

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

@alextwoods
alextwoods requested a review from a team as a code owner September 22, 2026 20:20
@alextwoods alextwoods added the no-api-surface-area-change Indicate there is no API surface area change and thus API surface area review is not required label Sep 24, 2026
@alextwoods
alextwoods marked this pull request as draft September 24, 2026 17:32
@alextwoods
alextwoods force-pushed the alexwoo/apache_panic branch from 7c9dffe to 49c6f57 Compare October 15, 2026 21:50

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-api-surface-area-change Indicate there is no API surface area change and thus API surface area review is not required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant