Skip to content

OPENNLP-1948: Release file handles by MarkableFileInputStreamFactory and the Leipzig reader on close and reset - #1297

Merged
mawiesne merged 1 commit into
apache:mainfrom
ai-pipestream:OPENNLP-1948-file-handle-leaks
Sep 16, 2026
Merged

mawiesne merged 1 commit into
apache:mainfrom
ai-pipestream:OPENNLP-1948-file-handle-leaks

Conversation

@krickert

Copy link
Copy Markdown
Contributor

Three readers kept a file descriptor open until the garbage collector ran. On main, 200 open-and-close cycles of MarkableFileInputStreamFactory.createInputStream() left 200 descriptors open. Long training or cross-validation runs over many files can run into "Too many open files", and on Windows the files stay locked.

Changes

  • MarkableFileInputStream (opennlp-formats, opennlp.tools.util): close() now closes the FileInputStream the class opened. Before, the inherited InputStream.close() was a no-op, so closing the stream, or a reader wrapped around it, never closed the file. Closing twice has no effect.
  • LeipzigLanguageSampleStream.LeipzigSentencesStream: the Files.lines stream used to count the lines of a sentence file is closed after counting. Before, it was never closed.
  • PlainTextByLineStream.reset() (opennlp-api): the previous reader is closed before a new one is created. Before, each reset opened a new input stream and dropped the old one unclosed, so every pass over a MarkableFileInputStreamFactory leaked one descriptor.

No behavior changes beyond releasing the descriptors: the bytes read, the lines returned, and the samples produced are the same.

Tests

  • MarkableFileInputStreamTest (new): mark and reset, reset without mark rejected, a read after close() fails, close twice accepted, and closing a BufferedReader over the stream closes the file. The two close tests fail on main.
  • PlainTextByLineStreamTest: a factory hands out streams that record close(); after each reset() the earlier stream is closed and the current one open, and close() closes the last one. Fails on main.
  • LeipzigLanguageSampleStreamTest: the sample directory is read a second time with the same result; the descriptor released by the fix is not observable portably, so the test pins the behavior around it.

Full runs with checkstyle, -Dopennlp.forkCount=1: opennlp-api 385, opennlp-formats 363, opennlp-tools 419 tests, 0 failures.

Manual

introduction.xml: a new section "Closing and resetting input streams" states that reset() closes the previous reader, so an InputStreamFactory must hand out a fresh stream for each pass, and names the two other fixes.

Release note and backport

All three leaks exist on 2.x (opennlp-tools). This change is meant for a release note and a backport to the 2.x line. The same fixes for the first two leaks are on the #1279 branch (OPENNLP-1933) and come off it once this one is in.

OPENNLP-1948

MarkableFileInputStream closes the file it opened, the Leipzig line count
closes its stream, and PlainTextByLineStream.reset closes the previous reader
before opening the next one. Contract tests cover each. The manual states the
reset rule for InputStreamFactory implementations.
@krickert
krickert force-pushed the OPENNLP-1948-file-handle-leaks branch from 6944822 to bf7c5aa Compare September 16, 2026 02:30
@rzo1
rzo1 requested a review from mawiesne September 16, 2026 05:24

@mawiesne mawiesne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, +1

@mawiesne
mawiesne marked this pull request as ready for review September 16, 2026 05:38
@mawiesne mawiesne added java Pull requests that update Java code tests Pull requests that add or update test code labels Sep 16, 2026
@mawiesne

Copy link
Copy Markdown
Contributor

@krickert Can you tell the team to provide a backport of this PR to opennlp-2.x, as separate PR?

@mawiesne mawiesne changed the title OPENNLP-1948: Release file handles on close and reset OPENNLP-1948: Release file handles by MarkableFileInputStreamFactory and the Leipzig reader on close and reset Sep 16, 2026
@mawiesne
mawiesne merged commit a00134c into apache:main Sep 16, 2026
10 checks passed
@krickert
krickert deleted the OPENNLP-1948-file-handle-leaks branch September 16, 2026 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

java Pull requests that update Java code tests Pull requests that add or update test code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants