Skip to content

fix(core): Disable manifest URL caching when reading versions (JAVA-730) - #6124

Open
0xadam-brown wants to merge 1 commit into
mainfrom
fix/manifest-version-reader-leak
Open

0xadam-brown wants to merge 1 commit into
mainfrom
fix/manifest-version-reader-leak

Conversation

@0xadam-brown

@0xadam-brown 0xadam-brown commented Sep 16, 2026

Copy link
Copy Markdown
Member

📜 Description

ManifestVersionReader was retaining jar-backed inflater state while scanning META-INF/MANIFEST.MF entries. PR fixes that by disabling URL caching for those reads and close the stream after parsing.

💡 Motivation and Context

Note that the ManifestVersionReader is invoked during Sentry.init(), but only on the JVM (not on Android). Performance penalty for the fix is limited:

  • scan only runs once per process;
  • it already enumerates every META-INF/MANIFEST.MF on the classpath (which is much more costly than the relatively tiny difference btw cached vs uncached access); and
  • it's used during version detection and not on a hot request path.

resolves: JAVA-730

💚 How did you test it?

I had my clanker reproduce the issue, then fix, then verify we were no longer leaking the jars.

No tests added because ManifestVersionReader is currently without a test suite, and adding it would require introducing a new constructor with a ClassLoader argument. Happy to do so if folks prefer, but for now I've kept the change minimal.

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.

🔮 Next steps

@linear-code

linear-code Bot commented Sep 16, 2026

Copy link
Copy Markdown

JAVA-730

ClassLoader.getSystemClassLoader().getResources("META-INF/MANIFEST.MF");
while (resources.hasMoreElements()) {
try {
final @NotNull Manifest manifest = new Manifest(resources.nextElement().openStream());

@0xadam-brown 0xadam-brown Sep 16, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard to see what happened b/c the diff realigns the indententation for most of this method. Here's what changed:

Before

final @NotNull Manifest manifest = new Manifest(resources.nextElement().openStream());
...

After

 final @NotNull URLConnection connection = resources.nextElement().openConnection();
     connection.setUseCaches(false);
     try (final @NotNull InputStream inputStream = connection.getInputStream()) {
        ...
     }

Note that simply closing the stream was insufficient to avoid leaking the jars. Verified with my clanker that disabling caches was also needed.

@sentry

sentry Bot commented Sep 16, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.57.0 (1) release

⚙️ sentry-android Build Distribution Settings

ManifestVersionReader was retaining jar-backed inflater state while scanning META-INF/MANIFEST.MF entries. Disable URL caching for those reads and close the stream after parsing.
@0xadam-brown
0xadam-brown force-pushed the fix/manifest-version-reader-leak branch from a4d0445 to 1bb7493 Compare September 16, 2026 15:38
@0xadam-brown
0xadam-brown marked this pull request as ready for review September 16, 2026 15:38
@0xadam-brown 0xadam-brown added the sanity-check PR needs a lightweight review for obvious issues label Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sanity-check PR needs a lightweight review for obvious issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant