Skip to content

fix(core): Close manifest streams after version detection - #6125

Closed
adinauer wants to merge 3 commits into
mainfrom
fix/close-manifest-streams
Closed

adinauer wants to merge 3 commits into
mainfrom
fix/close-manifest-streams

Conversation

@adinauer

Copy link
Copy Markdown
Member

📜 Description

  • Close every manifest input stream after ManifestVersionReader finishes parsing it, including malformed manifests.
  • Disable URL connection caching before opening each stream so jar: connections release their JarFile and inflater resources when closed.
  • Add regression tests for stream closure, cache configuration order, parse failures, and continued manifest scanning.

💡 Motivation and Context

ManifestVersionReader passed the result of URL.openStream() directly to Manifest, which does not close the supplied stream. This could retain a stream and associated JAR resources for every manifest scanned during SDK initialization.

resolves: #6120

💚 How did you test it?

  • ./gradlew :sentry:test — 3,550 tests passed
  • ./gradlew spotlessApply apiDump
  • SDK team review — approved with no P0, P1, or P2 findings

📝 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

None.

adinauer and others added 2 commits September 17, 2026 06:16
Disable URL connection caching and close each manifest input stream after parsing to avoid retaining JAR resources during SDK initialization. Add coverage for successful and malformed manifests.

Fixes GH-6120

Co-Authored-By: Claude <noreply@anthropic.com>
@sentry

sentry Bot commented Sep 17, 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

this(ClassLoader.getSystemClassLoader());
}

ManifestVersionReader(final @NotNull ClassLoader classLoader) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this constructor allows the creation of mutliple instances of what was previously a lazily instantiated singleton.

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.

This is just for tests, I'll mark it accordingly.

Mark the injected class loader constructor as test-only and document why manifest URL connection caching must remain disabled.

Refs GH-6120

Co-Authored-By: Claude <noreply@anthropic.com>
@0xadam-brown

0xadam-brown commented Sep 18, 2026

Copy link
Copy Markdown
Member

Closing as it duplicates #6124.

(Obviously feel free to follow up with the tests if you care to @adinauer 👍 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ManifestVersionReader leaks a ZipFile Inflater per JAR (openStream never closed)

3 participants