newrelic / newrelic-java-agent

The New Relic Java agent
Apache License 2.0
202 stars 144 forks source link

Edge case: rare memory leak when using legacy Async API #1554

Closed jasonjkeller closed 11 months ago

jasonjkeller commented 1 year ago

In rare cases, it is possible for a memory leak to manifest when using the agent's legacy async API which manages the suspend/resume/complete lifecycle of Jetty Continuations or the Async Servlet API.

The issue could occur when the agent creates and suspends multiple transactions for a single server request (Jetty, when combined with Karaf and Camel CXF, exhibited such behavior via multiple calls to ContextHandler.doHandle creating/suspending multiple transactions).

Essentially, an AsyncContext instance (for a given Request) is used as a key that maps to a Transaction instance that is being suspended (i.e. asyncTransactions.put(asyncContext, currentTxn)) and when the suspended Request is resumed the AsyncContext is used to retrieve the Transaction and resume it on the current thread and allow it to complete. The memory leak happens when the same AsyncContext instance is used to map to multiple Transaction instance, the latter of which gets overwritten each time the map is updated. This means that transactions that were suspended and stored in the map, but then overwritten, will never be resumed and completed, instead being held in memory indefinitely.

https://github.com/newrelic/newrelic-java-agent/blob/94b344ab58e4f49b7a5d9e8b90b26be87f249738/newrelic-agent/src/main/java/com/newrelic/agent/AsyncApiImpl.java#L35-L51

Even if we allowed for a single AsyncContext instance to map to multiple suspended Transaction instances it does not seem possible to actually resume more than one Transaction on a given thread as resuming a Transaction involves setting it as the active instance in the ThreadLocal.

workato-integration[bot] commented 1 year ago

https://new-relic.atlassian.net/browse/NR-171475