Re: WIP Incremental JSON Parser
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Dunstan <andrew@dunslane.net>
Cc: Nathan Bossart <nathandbossart@gmail.com>,
Jelte Fennema-Nio <postgres@jeltef.nl>,
Amit Langote <amitlangote09@gmail.com>,
Jacob Champion <jacob.champion@enterprisedb.com>,
Robert Haas <robertmhaas@gmail.com>,
Peter Smith <smithpb2250@gmail.com>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-04-08T00:58:36Z
Lists: pgsql-hackers
Coverity complained that this patch leaks memory: /srv/coverity/git/pgsql-git/postgresql/src/bin/pg_combinebackup/load_manifest.c: 212 in load_backup_manifest() 206 bytes_left -= rc; 207 json_parse_manifest_incremental_chunk( 208 inc_state, buffer, rc, bytes_left == 0); 209 } 210 211 close(fd); >>> CID 1596259: (RESOURCE_LEAK) >>> Variable "inc_state" going out of scope leaks the storage it points to. 212 } 213 214 /* All done. */ 215 pfree(buffer); 216 return result; 217 } /srv/coverity/git/pgsql-git/postgresql/src/bin/pg_verifybackup/pg_verifybackup.c: 488 in parse_manifest_file() 482 bytes_left -= rc; 483 json_parse_manifest_incremental_chunk( 484 inc_state, buffer, rc, bytes_left == 0); 485 } 486 487 close(fd); >>> CID 1596257: (RESOURCE_LEAK) >>> Variable "inc_state" going out of scope leaks the storage it points to. 488 } 489 490 /* Done with the buffer. */ 491 pfree(buffer); 492 493 return result; It's right about that AFAICS, and not only is the "inc_state" itself leaked but so is its assorted infrastructure. Perhaps we don't care too much about that in the existing applications, but ISTM that isn't going to be a tenable assumption across the board. Shouldn't there be a "json_parse_manifest_incremental_shutdown()" or the like to deallocate all the storage allocated by the parser? regards, tom lane
Commits
-
Post review fixes for test_json_parser test module
- ba3e6e2bca97 17.0 landed
-
Shrink test file for test_json_parser module
- b8a7bfa33324 17.0 landed
-
Add support for incrementally parsing backup manifests
- ea7b4e9a2a7c 17.0 landed
-
Introduce a non-recursive JSON parser
- 3311ea86edc7 17.0 landed
-
Use incremental parsing of backup manifests.
- 222e11a10ae9 17.0 landed