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: Threading issues in binary image cache #3468

Merged
merged 4 commits into from
Dec 1, 2023

Conversation

philipphofmann
Copy link
Member

📜 Description

Add synchronize to start and stop of SentryBinaryImage crash to avoid threading issues that can lead to crashes.

💡 Motivation and Context

Fixes GH-3462

💚 How did you test it?

Unit tests.

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • 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.
  • 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.

🔮 Next steps

Add synchronize to start and stop of SentryBinaryImage crash to avoid
threading issues that can lead to crashes.

Fixes GH-3462
Copy link

codecov bot commented Nov 30, 2023

Codecov Report

Merging #3468 (5139041) into main (4fd1a5d) will decrease coverage by 0.004%.
The diff coverage is 100.000%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3468       +/-   ##
=============================================
- Coverage   88.965%   88.961%   -0.004%     
=============================================
  Files          525       525               
  Lines        56638     56685       +47     
  Branches     20384     20399       +15     
=============================================
+ Hits         50388     50428       +40     
- Misses        5329      5335        +6     
- Partials       921       922        +1     
Files Coverage Δ
Sources/Sentry/SentryBinaryImageCache.m 100.000% <100.000%> (ø)
...ests/SentryTests/SentryBinaryImageCacheTests.swift 100.000% <100.000%> (ø)

... and 13 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4fd1a5d...5139041. Read the comment docs.

@philipphofmann philipphofmann merged commit 5ab8ec2 into main Dec 1, 2023
43 of 45 checks passed
@philipphofmann philipphofmann deleted the fix/threading-issues-for-image-cache branch December 1, 2023 07:24
Copy link

github-actions bot commented Dec 1, 2023

Fails
🚫 Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Threading issues in binary image cache ([#3468](https://github.com/getsentry/sentry-cocoa/pull/3468))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against f656c2c

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.

Crashed when SentrySDK.start in iOS
2 participants