Guard ManagedConnection cleanup against a torn-down physical connection#26104
Open
renatsaf wants to merge 1 commit into
Open
Guard ManagedConnection cleanup against a torn-down physical connection#26104renatsaf wants to merge 1 commit into
renatsaf wants to merge 1 commit into
Conversation
cleanup() -> resetConnectionProperties() resets the transaction isolation level and auto-commit value so a pooled connection can be safely reused. resetIsolation() re-fetches the physical connection via getActualConnection() and tolerates a null result, but resetAutoCommit() dereferenced actualConnection directly. When cleanup() runs on a pooled/XA ManagedConnection whose physical connection has already been torn down - for instance one marked for removal whose transaction completed and which was then destroyed - both actualConnection and pooledConnection are null. resetAutoCommit() then threw a NullPointerException. Guard resetConnectionProperties() with the same null-connection check resetIsolation() already relies on, so cleanup becomes a no-op when there is no physical connection to reset. Adds a regression test that reproduces the NPE on the reported code path. Fixes eclipse-ee4j#24232 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ManagedConnectionImpl.cleanup()→resetConnectionProperties()resets the transaction isolation level and the auto-commit value so a pooled connection can be safely returned to the pool.ManagedConnectionFactoryImpl.resetIsolation()re-fetches the physical connection throughgetActualConnection()and already tolerates anullresult, butresetAutoCommit()dereferencedactualConnectiondirectly.When
cleanup()runs on a pooled/XAManagedConnectionwhose physical connection has already been torn down — for instance one marked for removal whose transaction completed (transactionCompleted()nullsactualConnection) and which was then destroyed (destroy()nullspooledConnection) — both fields arenull.resetAutoCommit()then throws:This is the same code path reported in #24232. The originally reported NPE in
getActualConnection()itself was already fixed by an earlier guard; this addresses the remaining latent NPE inresetAutoCommit()on the same scenario.Fix
Guard
resetConnectionProperties()with the same null-connection checkresetIsolation()already relies on. WhengetActualConnection()returnsnullthere is no physical connection to reset, so cleanup becomes a no-op instead of dereferencing a null connection.Test
Adds
ManagedConnectionImplTest.testCleanupOnTornDownPooledConnectionDoesNotThrow, which destroys a pooledManagedConnection(nulling both connection fields) after the application changed auto-commit, then callscleanup(). The test fails with the exact reported NPE onmainand passes with this change.Fixes #24232
🤖 Generated with Claude Code