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: Crash in Client when reading integrations #2398

Merged
merged 11 commits into from
Nov 18, 2022

Conversation

philipphofmann
Copy link
Member

📜 Description

Fix reading from the integrations list while it's being modified on another thread on the client.

💡 Motivation and Context

Fixes GH-2397

💚 How did you test it?

Unit tests.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

🔮 Next steps

Fix reading from the integrations list while it's being modified
on another thread on the client.

Fixes GH-2397
@github-actions
Copy link

github-actions bot commented Nov 16, 2022

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1210.00 ms 1242.94 ms 32.94 ms
Size 20.75 KiB 374.73 KiB 353.97 KiB

Baseline results on branch: master

Startup times

Revision Plain With Sentry Diff
0fdf0b2 1266.27 ms 1277.90 ms 11.63 ms
b15627c 1256.48 ms 1264.68 ms 8.20 ms
d883642 1215.39 ms 1242.78 ms 27.39 ms
202334c 1265.41 ms 1277.34 ms 11.93 ms
58f558d 1216.15 ms 1246.22 ms 30.07 ms
e2a3f3e 1224.06 ms 1248.20 ms 24.14 ms
0fdf0b2 1245.88 ms 1247.69 ms 1.82 ms
347a8e9 1243.50 ms 1265.90 ms 22.40 ms
791123d 1217.52 ms 1253.08 ms 35.56 ms
e2f1150 1220.39 ms 1249.94 ms 29.55 ms

App size

Revision Plain With Sentry Diff
0fdf0b2 20.51 KiB 332.90 KiB 312.39 KiB
b15627c 20.50 KiB 337.76 KiB 317.25 KiB
d883642 20.50 KiB 338.99 KiB 318.49 KiB
202334c 20.51 KiB 331.79 KiB 311.28 KiB
58f558d 20.50 KiB 338.99 KiB 318.49 KiB
e2a3f3e 20.75 KiB 368.01 KiB 347.26 KiB
0fdf0b2 20.51 KiB 332.90 KiB 312.39 KiB
347a8e9 20.51 KiB 333.16 KiB 312.65 KiB
791123d 20.51 KiB 331.81 KiB 311.30 KiB
e2f1150 20.51 KiB 333.10 KiB 312.59 KiB

Previous results on branch: fix/crash-when-iterating-integrations

Startup times

Revision Plain With Sentry Diff
e581bc2 1222.35 ms 1264.88 ms 42.53 ms
555ebde 1203.41 ms 1230.59 ms 27.18 ms
626d83f 1207.94 ms 1238.68 ms 30.74 ms
77acb59 1243.71 ms 1266.74 ms 23.03 ms
2298180 1250.60 ms 1270.64 ms 20.04 ms

App size

Revision Plain With Sentry Diff
e581bc2 20.75 KiB 374.73 KiB 353.97 KiB
555ebde 20.75 KiB 374.38 KiB 353.62 KiB
626d83f 20.75 KiB 374.73 KiB 353.97 KiB
77acb59 20.75 KiB 374.72 KiB 353.97 KiB
2298180 20.75 KiB 374.72 KiB 353.97 KiB

@github-actions
Copy link

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1246.02 ms 1248.67 ms 2.65 ms
Size 20.75 KiB 374.38 KiB 353.62 KiB

Baseline results on branch: master

Startup times

Revision Plain With Sentry Diff
f444dc4 1237.39 ms 1255.42 ms 18.03 ms
d73ebd0 1200.83 ms 1236.10 ms 35.27 ms
791123d 1256.10 ms 1266.68 ms 10.58 ms
4e037c4 1233.35 ms 1254.55 ms 21.20 ms
3a1d58d 1223.98 ms 1242.88 ms 18.90 ms
9ad4a5f 1251.53 ms 1276.46 ms 24.93 ms
411a940 1232.33 ms 1259.33 ms 27.01 ms
0e2e593 1234.78 ms 1266.10 ms 31.32 ms
f444dc4 1242.55 ms 1248.82 ms 6.27 ms
0fdf0b2 1243.92 ms 1250.86 ms 6.94 ms

App size

Revision Plain With Sentry Diff
f444dc4 20.75 KiB 367.00 KiB 346.24 KiB
d73ebd0 20.50 KiB 365.18 KiB 344.68 KiB
791123d 20.51 KiB 331.81 KiB 311.30 KiB
4e037c4 20.50 KiB 361.79 KiB 341.29 KiB
3a1d58d 20.75 KiB 374.35 KiB 353.60 KiB
9ad4a5f 20.50 KiB 342.23 KiB 321.73 KiB
411a940 20.50 KiB 361.79 KiB 341.29 KiB
0e2e593 20.50 KiB 333.88 KiB 313.38 KiB
f444dc4 20.75 KiB 367.00 KiB 346.25 KiB
0fdf0b2 20.51 KiB 332.90 KiB 312.39 KiB

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

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

LGTM

Sources/Sentry/SentryHub.m Show resolved Hide resolved
@philipphofmann philipphofmann merged commit d975745 into master Nov 18, 2022
@philipphofmann philipphofmann deleted the fix/crash-when-iterating-integrations branch November 18, 2022 08:54
kevinrenskers added a commit that referenced this pull request Nov 22, 2022
* 8.0.0:
  ref: Fix typos in OOMTracker (#2431)
  ref: Make SpanProtocol.data non nullable (#2409)
  ref: add/improve logging (#2420)
  ref: bump supported OS versions (#2414)
  test: shorten some tests (#2428)
  ref: Remove `- [SentryOptions initWithDict:didFailWithError:]` (#2404)
  ref: Mark [SpanProtocol setExtraValue:forKey] as deprecated (#2413)
  typos (#2421)
  test: Disable NSDataTracker in clearTestState (#2418)
  Update CHANGELOG.md (#2415)
  feat: Properly demangle Swift class name (#2162)
  chore: Create 8.0.0 branch
  test: include Sentry changes in hash keys (#2412)
  release: 7.31.2
  fix: Crash in Client when reading integrations (#2398)
  test: tooling improvements (#2400)
  fix: Don't increase session's error count for dropped events (#2374)
  Update CHANGELOG.md (#2396)
  release: 7.31.1
  Fix: Set the correct OOM event timestamp (#2394)

# Conflicts:
#	Sentry.xcodeproj/project.pbxproj
#	SentryPrivate.podspec
#	scripts/add-sentry-to-vlc.patch
kevinrenskers added a commit that referenced this pull request Nov 24, 2022
* master: (56 commits)
  meta: disable swiftlint file length check in TBDBClient.swift (#2435)
  Revert "test: shorten some tests (#2422)" (#2427)
  test: shorten some tests (#2422)
  test: include Sentry changes in hash keys (#2412)
  release: 7.31.2
  fix: Crash in Client when reading integrations (#2398)
  test: tooling improvements (#2400)
  fix: Don't increase session's error count for dropped events (#2374)
  Update CHANGELOG.md (#2396)
  release: 7.31.1
  Fix: Set the correct OOM event timestamp (#2394)
  test: Fix SentrySerializationTests.testSerializationFailsWithInvalidJSONObject (#2392)
  feat(hybrid-sdks): Add captureScreenshots to PrivateSDKOnly (#2384)
  ci: Increase test timeout for iOS 12 (#2391)
  test: Disable flaky testCrashReportCount1 (#2389)
  test: Disable flaky testSerializeWithUnderlyingNSException (#2387)
  build(deps): bump github/codeql-action from 2.1.31 to 2.1.32 (#2386)
  build(deps): bump fastlane from 2.210.1 to 2.211.0 (#2385)
  ref: json serialization error reporting (#2355)
  release: 7.31.0
  ...
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.

Crash in SentryClient.m line 644 (sentry & sentry_flutter) during app startup
3 participants