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 a data race for SentryId.empty #3072

Merged
merged 3 commits into from
Jun 5, 2023

Conversation

philipphofmann
Copy link
Member

📜 Description

Initialize the SentryId.empty in the class initializer to fix a data race.

💡 Motivation and Context

Came up in #3069.

💚 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

Initialize the SentryId.empty in the class initializer to fix a data race.
@github-actions
Copy link

github-actions bot commented May 30, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1199.39 ms 1212.00 ms 12.61 ms
Size 20.76 KiB 435.35 KiB 414.59 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d3abae0 1200.36 ms 1224.22 ms 23.87 ms
b9b0f0a 1239.76 ms 1257.68 ms 17.92 ms
154f795 1250.38 ms 1274.54 ms 24.16 ms
56ec5d0 1236.65 ms 1261.90 ms 25.25 ms
98a8c16 1206.40 ms 1232.14 ms 25.74 ms
25a5e8b 1235.16 ms 1255.47 ms 20.30 ms
b1b7d72 1246.31 ms 1261.00 ms 14.69 ms
e1eed6b 1259.08 ms 1270.57 ms 11.49 ms
25bcc50 1240.47 ms 1254.70 ms 14.23 ms
302ee8b 1194.02 ms 1223.34 ms 29.32 ms

App size

Revision Plain With Sentry Diff
d3abae0 20.76 KiB 434.92 KiB 414.16 KiB
b9b0f0a 20.76 KiB 434.94 KiB 414.18 KiB
154f795 20.76 KiB 435.25 KiB 414.49 KiB
56ec5d0 20.76 KiB 414.44 KiB 393.69 KiB
98a8c16 20.76 KiB 431.00 KiB 410.24 KiB
25a5e8b 20.76 KiB 436.33 KiB 415.57 KiB
b1b7d72 20.76 KiB 436.33 KiB 415.57 KiB
e1eed6b 20.76 KiB 432.17 KiB 411.41 KiB
25bcc50 20.76 KiB 427.22 KiB 406.46 KiB
302ee8b 20.76 KiB 419.62 KiB 398.87 KiB

Previous results on branch: fix/sentry-id-empty-data-race

Startup times

Revision Plain With Sentry Diff
0c99bb6 1204.65 ms 1233.14 ms 28.49 ms

App size

Revision Plain With Sentry Diff
0c99bb6 20.76 KiB 434.98 KiB 414.22 KiB

@codecov
Copy link

codecov bot commented May 30, 2023

Codecov Report

Merging #3072 (b6e189a) into main (154f795) will decrease coverage by 0.060%.
The diff coverage is 100.000%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3072       +/-   ##
=============================================
- Coverage   88.840%   88.780%   -0.060%     
=============================================
  Files          493       493               
  Lines        53164     53148       -16     
  Branches     19049     19042        -7     
=============================================
- Hits         47231     47185       -46     
- Misses        4971      4996       +25     
- Partials       962       967        +5     
Impacted Files Coverage Δ
Sources/Sentry/SentryId.m 100.000% <100.000%> (ø)
Tests/SentryTests/Protocol/SentryIdTests.swift 100.000% <100.000%> (ø)

... and 12 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 154f795...b6e189a. Read the comment docs.

@philipphofmann philipphofmann merged commit f0283e8 into main Jun 5, 2023
@philipphofmann philipphofmann deleted the fix/sentry-id-empty-data-race branch June 5, 2023 06:38
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.

2 participants