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 rare cases of corrupt feature flags in NDK reports #1801

Merged
merged 10 commits into from Feb 7, 2023

Conversation

lemnik
Copy link
Contributor

@lemnik lemnik commented Feb 7, 2023

Goal

Avoid reporting feature flags that were corrupted by being modified when a native crash was in progress.

Design

Added a new spin lock structure similar to a sequence-lock to protect the NDK-side feature flags. When altering feature flags its counter value will be odd (strictly (counter & 1) != 0) and when no changes are taking place the value will be even (strictly (counter & 1) == 0).

The signal handler takes a snapshot of the counter before and after writing the feature flags to the event dump. These two values are compared and a "verification" flag of one byte is written. If this value is zero (invalid) when loading the feature flags, all feature flags for the event are discarded (as they will have been corrupted).

This approach avoids the signal handler possibly having to wait on the lock, and avoids deadlocks.

Testing

Re-tested using a local saturation test.

@lemnik lemnik requested a review from kstenerud February 7, 2023 12:42
@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Feb 7, 2023

Android notifier sizes

Format Size impact of Bugsnag (kB) Size impact of Bugsnag when Minified (kB)
APK 1920.04 1695.49
arm64_v8a 680.33 459.15
armeabi_v7a 614.8 393.62
x86 754.04 532.86
x86_64 725.38 504.19

Generated by 🚫 Danger

@lemnik lemnik merged commit d28e909 into next Feb 7, 2023
@lemnik lemnik deleted the PLAT-9577/fix-corrupt-feature-flags branch February 7, 2023 14:38
This was referenced Feb 8, 2023
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.

None yet

3 participants