Skip to content

fix(core): fd leak in index writer initialization#6211

Merged
bluestreak01 merged 1 commit intomasterfrom
puzpuzpuz_bitmap_index_writer_fd_leak
Oct 1, 2025
Merged

fix(core): fd leak in index writer initialization#6211
bluestreak01 merged 1 commit intomasterfrom
puzpuzpuz_bitmap_index_writer_fd_leak

Conversation

@puzpuzpuz
Copy link
Copy Markdown
Contributor

@puzpuzpuz puzpuzpuz commented Oct 1, 2025 •
edited
Loading

Reproducible in WalWriterFuzzTest.testWalWriteTinyO3Memory with the following random seeds: 2574279280316L, 1759264733558L.

The problem was in these lines:

                kFdUnassigned = false;
                this.keyMem.of(ff, keyFd, null, ff.length(keyFd), MemoryTag.MMAP_INDEX_WRITER);

Here, the keyFd would leak due to the kFdUnassigned flag being set to false earlier in scenario when the ff.length(keyFd) call throws.

Also, reformats BitmapIndexWriter's code.

@puzpuzpuz puzpuzpuz self-assigned this Oct 1, 2025
@puzpuzpuz puzpuzpuz added Bug Incorrect or unexpected behavior Core Related to storage, data type, etc. labels Oct 1, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Oct 1, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch puzpuzpuz_bitmap_index_writer_fd_leak

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@glasstiger
Copy link
Copy Markdown
Contributor

glasstiger commented Oct 1, 2025

[PR Coverage check]

😍 pass : 30 / 35 (85.71%)

file detail

path covered line new line coverage
🔵 io/questdb/cairo/BitmapIndexWriter.java 30 35 85.71%

bluestreak01
bluestreak01 approved these changes Oct 1, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@bluestreak01 bluestreak01 left a comment

Choose a reason for hiding this comment

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

what is it with your dislike to this. ?

@bluestreak01 bluestreak01 merged commit f7c762d into master Oct 1, 2025
36 checks passed
@bluestreak01 bluestreak01 deleted the puzpuzpuz_bitmap_index_writer_fd_leak branch October 1, 2025 13:00
@puzpuzpuz
Copy link
Copy Markdown
Contributor Author

puzpuzpuz commented Oct 1, 2025

what is it with your dislike to this. ?

Its usage is inconsistent, even within a single method. Other than that, in most cases it's redudant and only makes the line longer and harder to read.

if (this.keyMem.getLong(BitmapIndexUtils.KEY_RESERVED_OFFSET_SEQUENCE_CHECK) != this.keyMem.getLong(BitmapIndexUtils.KEY_RESERVED_OFFSET_SEQUENCE)) {

@puzpuzpuz
Copy link
Copy Markdown
Contributor Author

puzpuzpuz commented Oct 1, 2025

@bluestreak01 thanks for the review!

bluestreak01 pushed a commit that referenced this pull request Oct 9, 2025
@puzpuzpuz @bluestreak01
fix(core): fd leak in index writer initialization (#6211)
c0ed602 
(cherry picked from commit f7c762d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bluestreak01 bluestreak01 bluestreak01 approved these changes

Assignees

@puzpuzpuz puzpuzpuz

Labels

Bug Incorrect or unexpected behavior Core Related to storage, data type, etc.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@puzpuzpuz @glasstiger @bluestreak01