Skip to content

fix(manifest): handle lifecycle of the decoder in reader#766

Merged
zeroshade merged 1 commit intoapache:mainfrom
ferhatelmas:manifest-reader-decoder-close
Mar 2, 2026
Merged

fix(manifest): handle lifecycle of the decoder in reader#766
zeroshade merged 1 commit intoapache:mainfrom
ferhatelmas:manifest-reader-decoder-close

Conversation

@ferhatelmas
Copy link
Contributor

related to #721

  • remove premature decoder close in the constructor so that reader can actually read the entries

  • add explicit close method for resource cleanup

  • call close in ReadManifest to prevent leak

  • add zstd codec based regression test

@ferhatelmas ferhatelmas changed the title fix(metadata): handle lifecycle of the code in reader-s fix(manifest): handle lifecycle of the code in reader-s Mar 2, 2026
@ferhatelmas ferhatelmas force-pushed the manifest-reader-decoder-close branch 2 times, most recently from a660257 to 86a61ce Compare March 2, 2026 22:06
@ferhatelmas ferhatelmas changed the title fix(manifest): handle lifecycle of the code in reader-s fix(manifest): handle lifecycle of the codec in reader Mar 2, 2026
@ferhatelmas ferhatelmas changed the title fix(manifest): handle lifecycle of the codec in reader fix(manifest): handle lifecycle of the decoder in reader Mar 2, 2026
related to apache#721

* remove premature decoder close in the constructor
so that reader can actually read the entries

* add explicit close method for resource cleanup

* call close in ReadManifest to prevent leak

* add zstd codec based regression test

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
@ferhatelmas ferhatelmas force-pushed the manifest-reader-decoder-close branch from 86a61ce to 9c4392c Compare March 2, 2026 22:08
Copy link
Member

@zeroshade zeroshade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good change! Thanks!

@zeroshade zeroshade merged commit f192c8e into apache:main Mar 2, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants