RFR: 8340814: ZipFileInflaterInputStream should release its Inflater after observing EOF
Eirik Bjørsnøs
eirbjo at openjdk.org
Mon Oct 7 20:02:10 UTC 2024
Please consider this PR which makes `ZipFileInflaterInputStream` release its `Inflater` instance back to the pool when observing that the stream has been fully consumed.
This is motivated by the following observations:
* After a `ZipFileInflaterInputStream` has reached the end of stream, its `Inflater` instance no longer does useful work.
* Obtaining an input stream via `ZipFile::getInputStream` without properly closing it is probably common. Obtaining it without fully consuming it is probably more rare. (Something like `classLoader.getInputStream(entry).readAllBytes()` is probably common)
* While GC will eventually release the `Inflater` when the `Cleaner` closes the stream, this will only happen at some later point in time. In the meantime, `ZipFile::getInputStream` may produce many new `Inflater` instances. These will all be released to the pool once GC eventually catches up.
* Once an `Inflater` is released to to pool, it will stay there for the lifetime of the `ZipFile`
* The lifetime of a `ZipFile` may often be as long as the application (consider class loaders).
This PR suggests the following changes:
* Rename the existing field `ZipFileInflaterInputStream.eof` to `compressedEof`. (This tracks the EOF of the filtered input stream)
* Add a new field `ZipFileInflaterInputStream.eof` to track the EOF of uncompressed data.
* Override `ZipFileInflaterInputStream.read(byte[] b, int off, int len)` to detect the EOF of uncompressed data and release the `Inflater` instance back to the pool when it is no longer needed.
* To protect the `Inflater` instance from being used after being released and reset, the following updates are needed:
* `ZipFileInflaterInputStream.read` must check if EOF has been reached but that the stream has not yet been closed. In this case, EOF should be returned.
* Similarly, `ZipFileInflaterInputStream.available` needs an update to return 0 if EOF has been detected.
The PR adds a test `ReleaseInflaterOnEOF` which verifies that all fully consumed input streams are backed by the same `Inflater` instance.
This PR was inspired by JDK-7031076 and this discussion: https://mail.openjdk.org/pipermail/core-libs-dev/2011-March/006341.html
-------------
Commit messages:
- Add comment justifying the override of InputStream.read
- Use correct copyright year for a new test
- Make sure the ZipFile is closed using TwR
- Call transferTo on the correct stream instance
- Make ZipFileInflaterInputStream release its Inflater to the pool when observing that the stream is fully consumed (EOF)
Changes: https://git.openjdk.org/jdk/pull/21157/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=21157&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8340814
Stats: 139 lines in 2 files changed: 135 ins; 0 del; 4 mod
Patch: https://git.openjdk.org/jdk/pull/21157.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/21157/head:pull/21157
PR: https://git.openjdk.org/jdk/pull/21157
More information about the core-libs-dev
mailing list