Fix deallocation of failed digested prepares - #2642
Open
AshSgDe29071999 wants to merge 1 commit into
Open
Conversation
When Prepare is called with name == sql (the stdlib path), the server-side statement is stmt_<digest> but a Describe-phase failure stored the SQL text. The next Prepare then Close'd a name that did not exist, leaked the digest statement, and retried with 42P05. Store the Parse name so cleanup targets the real server statement. See jackc#2640
|
Thanks for the PR, but I could have also let CC fix this in a second - I want to get feedback and see if this issue goes deeper or is this an issue on my implementation rather than a bug. @jackc this seem to solve the issue, but would be happy to hear your thoughts on the issue. i have created #2644 for this. this PR stops pointless cleanups from being scheduled at all, with tests that prove both the original issue and this |
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
2 participants
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.
Fixes #2640
database/sql/ GORM withPrepareStmt: truecallPrepare(ctx, sql, sql). In that path the wire name isstmt_<digest>while the client cache key is the SQL text.On a Describe-phase
PrepareError,failedDescribeStatementstored the cache key. The nextPrepareon that connection then calledDeallocatewith the SQL text. A failed prepare is never inpreparedStatements, soDeallocatesentClosefor a name that does not exist, the leakedstmt_<digest>stayed on the server, and a retry of the same SQL returned42P05for the rest of the connection's life. Cleanup errors also surfaced on whatever query next borrowed the pooled connection.Store
psName(the identifier actually sent in Parse) so the follow-upClosetargets the real server statement. The named-prepare path is unchanged becausepsName == psKeythere.The new test is the digest (
name == sql) variant ofTestPrepareHandlesTimeoutBetweenParseAndDescribe.