pjfanning opened a new pull request, #1254:
URL: https://github.com/apache/poi/pull/1254
Both `create(File, String)` and `create(InputStream, String)` guard their
cleanup with an inverted fatal check:
```java
} catch (Throwable t) {
if (!ExceptionUtil.isFatal(t)) {
fs.close();
throw t;
}
}
```
For a **fatal** throwable — `VirtualMachineError`, `ThreadDeath` — the catch
body does nothing at all. Execution falls through to the `if (extractor ==
null)` branch and the method returns `null`. So an `OutOfMemoryError` raised
while building the extractor is reported to the caller as "no extractor for
this file".
The rest of the codebase uses the opposite form, rethrowing fatal throwables
rather than suppressing them — `XMLHelper.java:177`, `CleanerUtil.java:184`,
`HSSFParser.java:53`.
### Fix
There is nothing here that needs to treat fatal throwables differently: the
only work in the catch block is closing a filesystem this method opened and
owns. So rather than correcting the condition to `if (ExceptionUtil.isFatal(t))
ExceptionUtil.rethrow(t);`, the check is dropped entirely and every throwable
propagates.
`fs.close()` also becomes `IOUtils.closeQuietly(fs)` so a failure while
closing cannot mask the original throwable —
`ExtractorFactory.createExtractor(File, String)` uses the same
closeQuietly-then-rethrow shape.
The filesystem is still closed on every failure path, so this does not
reintroduce the leak that the original catch block was there to prevent.
This came out of the same review as #1248 — #1253, but it is a correctness
bug rather than a leak, so it is separate.
No public signatures change, so there is nothing for MiMa to check.
`TestMainExtractorFactory` and `TestExtractorFactory` (120 tests) pass.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
