ramanathan1504 commented on PR #4127:
URL: https://github.com/apache/logging-log4j2/pull/4127#issuecomment-4512070153
@SebTardif
"Great work on this! Resource leaks in configuration loading can be
incredibly difficult to debug, especially in containerized or high-availability
environments. This is a solid improvement to Log4j's stability.
#### Suggestions for Completion:
To make this PR "merge-ready" for the standards, we should add a failure
test case and a changelog entry:
**1. Add a Failure Test Case**
Since the fix specifically targets the failure path (`!success`), we should
add a test that intentionally fails to find a file inside a JAR to ensure the
handle is released.
Add this to `ConfigurationSourceTest.java`:
```java
@Test
void testJarFileLeakOnFailure() throws Exception {
final Path jarFile = prepareJarConfigURL(tempDir);
// Path inside JAR that does NOT exist
final URL brokenJarConfigURL = new URL("jar:" + jarFile.toUri().toURL()
+ "!/missing/file.xml");
final long expectedFdCount = getOpenFileDescriptorCount();
// This triggers the FileNotFoundException internally
ConfigurationSource configSource =
ConfigurationSource.fromUri(brokenJarConfigURL.toURI());
assertNull(configSource, "Source should be null on failure");
// 1. Verify File Descriptors (Unix/Linux)
assertEquals(expectedFdCount, getOpenFileDescriptorCount(),
"File descriptors leaked after failed configuration load!");
// 2. Verify File Lock (Windows)
try {
Files.delete(jarFile);
} catch (IOException e) {
fail("Could not delete JAR file! A handle was leaked in the failure
path.");
}
}
```
**2. Add a Changelog Entry**
Log4j 2 uses an automated tool for release notes. Please create a new file:
`src/changelog/.2.x.x/fix_ConfigurationSource_leak.xml` :
```xml
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
xmlns="https://logging.apache.org/xml/ns";
xsi:schemaLocation="https://logging.apache.org/xml/ns
https://logging.apache.org/xml/ns/log4j-changelog-0.xsd";
type="fixed">
<issue id="4127"
link="https://github.com/apache/logging-log4j2/pull/4127"/>
<description format="markdown">
Fix resource leaks in ConfigurationSource when loading via URL
</description>
</entry>
```
Thanks again for the contribution! don't forgot to apply spotless !!😊
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
