Skip to content

fix: close streams in LoggerContextAdmin.setConfigLocationUri#4218

Open
SebTardif wants to merge 2 commits into
apache:2.xfrom
SebTardif:fix/f005-jmx-config-stream-leak
Open

fix: close streams in LoggerContextAdmin.setConfigLocationUri#4218
SebTardif wants to merge 2 commits into
apache:2.xfrom
SebTardif:fix/f005-jmx-config-stream-leak

Conversation

@SebTardif

Copy link
Copy Markdown
Contributor

What Problem This Solves

LoggerContextAdmin.setConfigLocationUri opened a FileInputStream or URL stream and passed it to ConfigurationSource(InputStream, File/URL). That constructor documents that the caller owns the stream. On some failure paths before the configuration parser closes the stream, the descriptor can leak. Same theme as the ConfigurationSource URL leak work in #4127.

Evidence

Failure scenario

configSource = new ConfigurationSource(new FileInputStream(configFile), configFile);
// if getConfiguration/start fails before the config impl closes the stream,
// the FileInputStream remains open

Fix

Buffer the configuration into memory with try-with-resources, then construct a ConfigurationSource(Source, byte[], lastModified) that owns a ByteArrayInputStream.

Red-green / tests

export JAVA_HOME=$(/usr/libexec/java_home -v 17)
./mvnw -pl log4j-core,log4j-core-test -am test \
  -Dtest=LoggerContextAdminSetConfigLocationUriTest \
  -Dsurefire.failIfNoSpecifiedTests=false

Result: 3 tests pass (valid file reconfigure, file: URL form, blank rejection).

Summary

  • Buffer + close file/URL streams in setConfigLocationUri
  • Regression tests in LoggerContextAdminSetConfigLocationUriTest
  • Changelog entry

ConfigurationSource(InputStream, File/URL) leaves stream ownership with the
caller. Buffer configuration bytes into a Source-backed ConfigurationSource
and close the FileInputStream/URL stream before ConfigurationFactory runs so
failed or successful JMX reconfigure paths do not leak descriptors.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant