fix(guideline): prevent zip file closed race condition in GuidelineIO#153
Merged
fix(guideline): prevent zip file closed race condition in GuidelineIO#153
Conversation
Contributor
|
From my side looks good? @XoMEX ? |
The previous fix disabled JarURLConnection caching to give each caller a private JarFile. That avoided the race but reopened and reparsed the jar on every readGuidelines call — costly under high-throughput scanning (millions of connections in the crawler). The cache itself was never the bug. The bug was wrapping the cached JarFile in try-with-resources and closing it from user code, which broke other threads still iterating the JVM-shared instance. JarFileFactory owns the lifecycle of cached JarFiles; callers must not close them. Drop setUseCaches(false) and the try-with-resources. Update the concurrency test docstring to reflect the new reasoning. The 50-thread × 200-iteration test continues to pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ic0ns
approved these changes
Apr 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #152
Problem
GuidelineIO.listXmlFilesopened the resource jar viaJarURLConnection.getJarFile()and wrapped the result in atry-with-resources. With the JVM's defaultuseCaches=true,getJarFile()returns a JVM-wide cachedJarFilefromJarFileFactory— shared across all callers. Closing it from one thread broke any other thread mid-iteration on the same instance:IllegalStateException: zip file closedFileNotFoundExceptionSee JDK-8246714.
Fix
Don't close the cached
JarFile. The JVM owns its lifecycle; user code must not callclose()on instances obtained from a cachedJarURLConnection.An earlier iteration of this PR set
useCaches(false)to give each caller a privateJarFile. That also fixed the race, but at the cost of reopening and reparsing the jar's central directory on everyreadGuidelinescall. Under high-throughput scanning (millions of connections in the crawler), that overhead is significant. The current fix keeps the JVM cache and just stops closing it — fast path preserved, race gone.Tests
GuidelineIOConcurrencyTest(new):testUnderlyingJdkCachingStillCausesErrorWithoutFix— demonstrates the JDK-level behaviour with a barrier-coordinated reproduction: two threads, one iterating, the other closing the shared cachedJarFile. The iterating thread throws. This documents whylistXmlFilesmust not close theJarFile.testConcurrentReadGuidelinesIsCorrectAfterFix— 50 threads × 200 iterations ofreadGuidelinesagainst a freshly-built test jar; asserts no exceptions and that every call returns the correct guideline count.Both pass. Existing
GuidelineIOTest(7 tests) also passes unchanged.