Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

fix: close streams in LoggerContextAdmin.setConfigLocationUri#4218

Open
SebTardif wants to merge 2 commits into
apache:2.xapache/logging-log4j2:2.xfrom
SebTardif:fix/f005-jmx-config-stream-leakSebTardif/logging-log4j2:fix/f005-jmx-config-stream-leakCopy head branch name to clipboard
Open

fix: close streams in LoggerContextAdmin.setConfigLocationUri#4218
SebTardif wants to merge 2 commits into
apache:2.xapache/logging-log4j2:2.xfrom
SebTardif:fix/f005-jmx-config-stream-leakSebTardif/logging-log4j2:fix/f005-jmx-config-stream-leakCopy head branch name to clipboard

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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.