Skip to content

seekable: fix seek table leaks on reinitialization and read failure - #4910

Closed
perfloop-agent wants to merge 1 commit into
meta:devfrom
perfloop-oss:perfloop-pr-open-3vc97h1sd7
Closed

perfloop-agent wants to merge 1 commit into
meta:devfrom
perfloop-oss:perfloop-pr-open-3vc97h1sd7

Conversation

@perfloop-agent

Copy link
Copy Markdown

What and why

ZSTD_seekable_loadSeekTable() now frees the installed entries immediately before replacing them with a fully loaded table. It also frees the newly allocated entries when a later refill read fails, returning the same I/O error.

This fixes lost seek-table allocations when a decoder is reused or a multi-read load fails. Failed loads keep the previously installed table's metadata, and exported seek tables remain independent copies. The initialization APIs' source-switching behavior is unchanged. Re-initialization frequency in applications has not been measured.

The test in contrib/seekable_format/tests/seekable_tests.c reuses a decoder across successful and failed loads, retries with a smaller archive, and checks an exported table after decoder destruction. It exercises refill failures with and without checksums.

The seekable tests passed in normal and AddressSanitizer/UndefinedBehaviorSanitizer builds. The parallel-compression consumer test also passed. Focused clang static analysis reported no bugs.

Checklist

  • I searched open pull requests and issues: this problem is not already being addressed.
  • Bug fix: it comes with a reproducer or a test that fails without the fix.
  • Performance change: numbers from repeated runs are included, with CPU, compiler, flags and data. Not applicable: correctness fix.
  • I stated below whether AI or other automated tools were used.

Tools used

AI tools were used to investigate ownership, implement the fix, and write the tests.

Case and evidence


Made by Perfloop. Human sponsor: Tomás Senart. To ask for a change, comment or review in plain words. How Perfloop handles pull requests.

Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Signed-off-by: Tomás Senart <ts@perfloop.ai>
@meta-cla

meta-cla Bot commented Nov 10, 2026

Copy link
Copy Markdown

Hi @perfloop-agent!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@tsenart

tsenart commented Nov 10, 2026

Copy link
Copy Markdown

Hi, I'm Tomás, I build Perfloop. I approved this contribution and will handle review feedback here.

@Cyan4973, this fixes two memory leaks in the same seekable-format module as #4902 and #4717. Successful reinitialization with ZSTD_seekable_initBuff, ZSTD_seekable_initFile or ZSTD_seekable_initAdvanced leaks the previous seek table, and a failed refill read after allocation leaks the new entries. This change frees the previous table when installing a replacement and frees the unpublished entries on refill failure. We found it while hunting correctness bugs with Perfloop. We've had changes merged in Redis, Dolt, Prometheus, ClickHouse, Distribution, and more.

If you'd like anything in this patch changed, just let me know and I'll update it.

@meta-cla meta-cla Bot added the CLA Signed label Nov 10, 2026
@meta-cla

meta-cla Bot commented Nov 10, 2026

Copy link
Copy Markdown

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@perfloop-agent

Copy link
Copy Markdown
Author

Closing in favor of #4941, which covers both leaks and the reset contract. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants