SebTardif commented on PR #4218: URL: https://github.com/apache/logging-log4j2/pull/4218#issuecomment-5161173859
@ramanathan1504 thanks for the detailed review — updated. 1. **Red-green test:** `setConfigLocationUri_closesCallerOwnedStreamWhenFactoryDoesNotConsumeIt` installs a `ConfigurationFactory` that returns `DefaultConfiguration` without reading the stream. Without the try-with-resources, open FD count rises (`expected: 173 but was: 177` locally). With the fix, FD count holds and the temp file can be deleted (Windows lock path). Happy-path tests still load real XML. 2. **No full buffering:** dropped the `toByteArray` / `Source`+`byte[]` approach. Source stays stream/file-backed so `resetInputStream()` still re-opens the file for `monitorInterval` watchers. 3. **Leak note:** Xml/JSON/Properties factories already close when they consume the stream. The gap is the **caller-owns-stream** contract when a factory path never consumes (or fails before consume). try-with-resources on the JMX method covers that without changing reconfigure/watch behavior. Ready for another look. -- 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]
