sql/parser: fix memory leak of scanned comments #127711
Conversation
mgartner
added
backport-23.2.x
mgartner
requested review from
a team and
michae2
and removed request for
a team
![blathers[crl]](https://avatars.githubusercontent.com/in/59700?s=40&u=373d9101a12edc3847d2fb6ae78e3adcfb4ca41e&v=4){kind=avatar}
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
There was a problem hiding this comment.
Nice, find!
Reviewed all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @michae2)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:
-- commits line 14 at r1:
nit: should we add a release note for it? It looks like this behavior has been present since forever22.2.
yuzefovich
added
the
backport-23.1.x
|
TFTRs! bors r+ |
|
bors r- |
|
Canceled. |
This commit fixes a memory leak that cause a session's `scanner.Scanner`, which is embedded in a `pgwire.conn` via a `parser.Parser`, to retain previously scanned SQL comments in all parsed SQL statements. The scanner's slice of comments would continue to grow as it parsed new SQL strings with comments, as long as the session remained open. Also, the slice of comments holds references to the bytes in `pgwire` read buffers, preventing those buffers from being GC'd. Fixes cockroachdb#127710 Release note (bug fix): A bug has been fixed that caused a memory leak when executing SQL statements with comments, for example, `SELECT /* comment */ 1;`. Memory owned by a SQL session would continue to grow as these types of statements were executed. The memory would only be released when closing the SQL session. This bug has been present since version 23.1.
mgartner
force-pushed
the
fix-comment-leak
branch
from
0186c32 to
6ea14db
Compare
Done. Looks like it made it into 23.1, but the backport to 22.2 was not merged: #92844. |
|
bors r+ |
|
Build failed (retrying...): |
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:
This commit fixes a memory leak that cause a session's
scanner.Scanner, which is embedded in apgwire.connvia aparser.Parser, to retain previously scanned SQL comments in all parsedSQL statements. The scanner's slice of comments would continue to grow
as it parsed new SQL strings with comments, as long as the session
remained open. Also, the slice of comments holds references to the bytes
in
pgwireread buffers, preventing those buffers from being GC'd.Fixes #127710
Release note (bug fix): A bug has been fixed that caused a memory leak
when executing SQL statements with comments, for example,
SELECT /* comment */ 1;. Memory owned by a SQL session would continueto grow as these types of statements were executed. The memory would
only be released when closing the SQL session. This bug has been present
since version 23.1.