Fix fdb_push bound-param leak in fdb_push_free - #6098
Conversation
9d3a3b9 to
4c7955e
Compare
dohsql_free_params(&n, &p, count) frees the first `count` params (while(count--)). fdb_push_free passed p->nparams-1, so the last bound param always leaked -- and with a single param (nparams==1) nothing was freed at all. Pass p->nparams to free all of them, and relax the abort guard from `index >= *pnparams` to `index > *pnparams` so freeing the full count is valid (the clone error path still passes a count < nparams). Adds a regression subtest to fdb_push.test: 100 remote push-inserts of a single 512KB bound blob, asserting the "uncategorized" memstat bucket (where dohsql clones params) does not balloon. Grows ~50MB before the fix, ~7KB after. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Emelia Lei <wlei29@bloomberg.net>
4c7955e to
8f73957
Compare
roborivers
left a comment
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Error ⚠.
Regression testing: Success ✓.
The first 10 failing tests are:
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**
roborivers
left a comment
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Error ⚠.
Regression testing: Success ✓.
The first 10 failing tests are:
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**
roborivers
left a comment
roborivers
left a comment
There was a problem hiding this comment.
Cbuild submission: Success ✓.
Regression testing: Success ✓.
The first 10 failing tests are:
sc_resume_logicalsc_generated **quarantined**
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**
dorinhogea
left a comment
dorinhogea
left a comment
There was a problem hiding this comment.
looks good to me, thank you
3 participants
What
Fixes the bound-param leak flagged in #6027 (comment).
dohsql_free_params(&n, &p, count)frees the firstcountparams (while(count--)).fdb_push_freepassedp->nparams-1, so the last bound param always leaked — and with a single param (nparams==1) nothing was freed at all.Fix
db/fdb_push.c: passp->nparams(free all) instead ofp->nparams-1.db/dohsql.c: relax the abort guardindex >= *pnparams→index > *pnparamsso freeing the full count is valid. The only other caller (the clone error path) passes a count< nparams, so it is unaffected.Test
tests/fdb_push.testgets a regression subtest (reusing the existing 3-db harness): 100 remote push-inserts of a single 512KB bound blob (nparams==1), asserting theuncategorizedmemstat bucket — where dohsql clones params — does not balloon.🤖 Generated with Claude Code