Skip to content

[#1078] Stop the MakeLDIF generator when a template import ends before its reader - #1082

Open
vharseko wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:issue-1078-makeldif-generator-leak
Open

vharseko wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:issue-1078-makeldif-generator-leak

Conversation

@vharseko

Copy link
Copy Markdown
Member

Fixes #1078

Problem

LDIFImportConfig(TemplateFile) starts the MakeLDIF generator thread as soon as it is built (MakeLDIFInputStream.newStartedInputStream()). That thread stops only when its stream is closed. LDIFImportConfig.close() reached the stream only through reader, and reader exists only once a backend calls getReader(). A template import which failed before that left MakeLDIF Input Stream Thread spinning in writeEntry() for the life of the server, holding its TemplateFile and its queue.

Fix

  • LDIFImportConfig keeps the stream it created itself in a separate field, ownedInputStream, and close() closes it. A stream handed to the config through LDIFImportConfig(InputStream) still belongs to the caller: the replication domain passes the stream a total update arrives on (LDAPReplicationDomain:5859), so the config does not close it. The template constructor delegates to that public constructor, so ldifInputStream alone cannot tell the two cases apart.
  • ImportTask now closes the config when the reject file cannot be opened. In [#1026] Close the import config on every path an import task can end on #1028 a close there was dead code, since no file is open on that road. For a template import, though, the generator is already running there, and that return comes before the try whose finally closes the config. Every other early road already goes through that finally.

Tests

  • LDIFImportConfigTestCase (new):
    • testClosingATemplateConfigWhichWasNeverReadStopsItsGenerator: 100 entries, well past the queue of ten, so the test first asserts the generator did not finish on its own.
    • testClosingAConfigLeavesOpenTheStreamItWasHanded.
  • TestImportAndExport.testTemplateImportWhichCannotOpenItsRejectFileStopsItsGenerator: a template import with a directory as the reject file. The generator is read from the task the scheduler kept, the same way [#1026] Close the import config on every path an import task can end on #1028 reads the config.

Measured locally:

run result
new tests against master both generator cases red: The generator is still running …
head: LDIFImportConfigTestCase, TestImportAndExport, MakeLDIFTestCase, TestLDIFReader 76 run, 0 failures
mutant: the new close() dropped from the reject catch only the TestImportAndExport template case red
mutant: close() closes ldifInputStream instead of the owned stream only testClosingAConfigLeavesOpenTheStreamItWasHanded red

Notes on the issue text

  • The leak needs a template of more than ten entries. With fewer, the generator fills the queue, finishes and exits.
  • import-ldif --templateFile failing offline does return the shell: every such road returns 1, and ImportLDIF.main calls System.exit on a non-zero code. Those roads do skip importConfig.close() (a skip-file failure leaks the reject writer, as [#1026] Close the import config on every path an import task can end on #1028 found for the task), but the process exits anyway. They are left out of this PR.

Not in this PR

In MakeLDIFInputStream, the fields closed, allGenerated and ioException are shared between threads without volatile, and writeEntry / getNextEntry swallow InterruptedException.

…te import ends before its reader

LDIFImportConfig(TemplateFile) starts the generator thread when it is built,
and the thread stops only when its stream is closed. close() reached that
stream only through the reader a backend takes, so an import which failed
before a backend asked for the reader left the thread running for the life
of the server. The config now closes the stream it created itself, and still
leaves open a stream it was handed. ImportTask closes the config on the
reject-file failure too: that return comes before the try whose finally
closes it.
@vharseko vharseko added bug java Changes to Java sources tests Test suites: fixing, enabling, un-disabling tasks Server administrative tasks: import, export, backup, restore labels Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug java Changes to Java sources tasks Server administrative tasks: import, export, backup, restore tests Test suites: fixing, enabling, un-disabling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A MakeLDIF import which fails before its reader leaves the generator thread running

1 participant