Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unlikely SpanTracker leak #205

Merged
merged 4 commits into from
Jan 26, 2024
Merged

Conversation

lemnik
Copy link
Contributor

@lemnik lemnik commented Jan 24, 2024

Goal

Avoid possibly leaking tracked spans due to garbage-collection of their tokens. When a tracked span is on the SpanContext stack, and its "token" object (Activity, Fragment, etc.) is garbage-collected without the span being ended, it will remain on the stack permanently (because the SpanTracker does not know to end it).

Design

The SpanTracker is now implemented as a special-purpose WeakHashMap implementation. When spans are "lost" due to their tokens being garbage collected, the spans are correctly ended and removed from the table. This implementation also avoids the "map in map" design of the previous SpanTracker by instead implementing a composite-key structure.

WeakHashMap itself is unfortunately unsuitable as we have no way to determine when entries are removed due to garbage collection.

Testing

Additional unit tests

@lemnik lemnik force-pushed the PLAT-10282/fix-unlikely-spantracker-leak branch from 1e8b71f to 3f3b02c Compare January 24, 2024 12:33
@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Jan 24, 2024

Android notifier sizes

Format Size impact of Bugsnag (kB)
APK 77.87
Minified APK 47.86

Generated by 🚫 Danger

…w Array<SpanBinding?>.unlinkBindingWhere function to simplify unlinking and make it more robust.
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM

@lemnik lemnik merged commit d02c55a into next Jan 26, 2024
13 checks passed
@lemnik lemnik deleted the PLAT-10282/fix-unlikely-spantracker-leak branch January 26, 2024 17:21
@YYChen01988 YYChen01988 mentioned this pull request Feb 22, 2024
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.

4 participants