Skip to content

[lumina] Release the vector file when FileBackedDataset fails to construct - #9481

Open
PDGGK wants to merge 1 commit into
apache:masterfrom
PDGGK:fix-lumina-dataset-leak
Open

[lumina] Release the vector file when FileBackedDataset fails to construct#9481
PDGGK wants to merge 1 commit into
apache:masterfrom
PDGGK:fix-lumina-dataset-leak

Conversation

@PDGGK

@PDGGK PDGGK commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Purpose

FileBackedDataset opens the vector file and then does work that can fail:

FileBackedDataset(File file, int dim, long totalCount, String phase, int bufferSize)
        throws IOException {
    this.raf = new RandomAccessFile(file, "r");
    this.channel = raf.getChannel();
    ...
    this.recordSizeInBytes = checkedRecordSize(dim, bufferSize);
    ...
    this.readBuf = ByteBuffer.allocateDirect(bufferSize);

checkedRecordSize throws IllegalStateException when a record does not fit the read buffer, and allocateDirect can fail on its own. Both run after the file is open.

Both call sites use try-with-resources:

try (FileBackedDataset dataset = new FileBackedDataset(tempVectorFile, dim, count, "pretrain")) {

but try-with-resources never receives an object whose constructor threw, so its close() never runs and the handle is stranded.

The enclosing writer already guards its own file this way at :112, so this brings the nested reader in line.

Tests

LuminaFileBackedDatasetCloseTest#testFailedConstructionReleasesTheFile runs 200 failed constructions with a dimension the buffer cannot hold, counting entries in /dev/fd before and after. It skips itself where that directory is not available.

Reverting the change turns it red: [200 failed constructions must not strand 200 descriptors].

A note on how the assertion got there, since the obvious versions do not work. Asserting that the exception is raised passes either way. Letting the descriptors run out does not work either — the RandomAccessFile finalizer releases them under GC pressure, so twenty thousand leaked handles still never exhaust the limit and the test stays green with the bug in place. Counting open descriptors directly is what actually separates the two.

mvn test -pl paimon-lumina passes; spotless and checkstyle clean.

…truct

FileBackedDataset opens a RandomAccessFile and then validates the record size
and allocates the read buffer, either of which can throw. A caller using
try-with-resources never receives an object whose constructor threw, so nothing
closes that handle.

The enclosing writer already guards its own file this way; this brings the
nested reader in line.
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.

1 participant