Skip to content

Conversation

itaybre
Copy link
Contributor

@itaybre itaybre commented Sep 10, 2025

Fixes: #6129

When dispatchAsyncOnMainQueue is called from the main thread, the block ends up being executed immediately instead of async. I am renaming the method to make the behavior clearer

#skip-changelog

Copy link

linear bot commented Sep 10, 2025

Copy link

codecov bot commented Sep 10, 2025

Codecov Report

❌ Patch coverage is 92.72727% with 4 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@4f5b2c9). Learn more about missing BASE report.
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...urces/Sentry/Profiling/SentryContinuousProfiler.mm 66.666% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             main     #6132   +/-   ##
========================================
  Coverage        ?   86.686%           
========================================
  Files           ?       435           
  Lines           ?     37074           
  Branches        ?     17398           
========================================
  Hits            ?     32138           
  Misses          ?      4891           
  Partials        ?        45           
Files with missing lines Coverage Δ
...ntryTestUtils/TestSentryDispatchQueueWrapper.swift 89.855% <100.000%> (ø)
Sources/Sentry/Profiling/SentryProfilerState.mm 99.328% <100.000%> (ø)
...ces/Sentry/Profiling/SentryProfilingSwiftHelpers.m 100.000% <100.000%> (ø)
Sources/Sentry/Profiling/SentryTraceProfiler.mm 95.238% <100.000%> (ø)
Sources/Sentry/SentryANRTrackerV1.m 99.212% <100.000%> (ø)
Sources/Sentry/SentryFramesTracker.m 100.000% <100.000%> (ø)
Sources/Sentry/SentrySDKInternal.m 87.126% <100.000%> (ø)
Sources/Sentry/SentrySubClassFinder.m 93.220% <100.000%> (ø)
Sources/Sentry/SentryTimeToDisplayTracker.m 100.000% <100.000%> (ø)
...urces/Sentry/_SentryDispatchQueueWrapperInternal.m 88.059% <100.000%> (ø)
... and 3 more

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 4f5b2c9...e292e7c. Read the comment docs.

Copy link
Contributor

github-actions bot commented Sep 10, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1219.38 ms 1244.49 ms 25.11 ms
Size 23.75 KiB 971.82 KiB 948.07 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
fd5961e 1210.59 ms 1235.57 ms 24.98 ms
f5d202b 1237.90 ms 1259.49 ms 21.59 ms
718c372 1220.09 ms 1235.15 ms 15.06 ms
52f3b6e 1219.57 ms 1239.70 ms 20.13 ms
37183fe 1212.33 ms 1238.92 ms 26.59 ms
d66f082 1227.08 ms 1247.04 ms 19.96 ms
1b9991e 1233.45 ms 1256.61 ms 23.17 ms
9be5373 1215.92 ms 1239.44 ms 23.52 ms
87fb58a 1233.12 ms 1257.17 ms 24.04 ms
5196f0d 1213.35 ms 1231.37 ms 18.02 ms

App size

Revision Plain With Sentry Diff
fd5961e 23.74 KiB 874.07 KiB 850.32 KiB
f5d202b 23.75 KiB 904.53 KiB 880.78 KiB
718c372 23.75 KiB 920.65 KiB 896.90 KiB
52f3b6e 23.75 KiB 920.54 KiB 896.79 KiB
37183fe 23.75 KiB 913.63 KiB 889.87 KiB
d66f082 23.75 KiB 928.85 KiB 905.10 KiB
1b9991e 23.75 KiB 908.01 KiB 884.26 KiB
9be5373 23.75 KiB 866.50 KiB 842.75 KiB
87fb58a 23.75 KiB 919.91 KiB 896.16 KiB
5196f0d 23.75 KiB 876.93 KiB 853.19 KiB

Previous results on branch: itay/cocoa-636-_sentrydispatchqueuewrapperinternaldispatchasynconmainqueue

Startup times

Revision Plain With Sentry Diff
25c4ec8 1228.96 ms 1242.88 ms 13.92 ms
248b305 1229.43 ms 1254.06 ms 24.64 ms
27a306a 1234.22 ms 1263.45 ms 29.22 ms

App size

Revision Plain With Sentry Diff
25c4ec8 23.75 KiB 969.21 KiB 945.46 KiB
248b305 23.75 KiB 969.26 KiB 945.51 KiB
27a306a 23.75 KiB 969.99 KiB 946.24 KiB

@itaybre itaybre marked this pull request as draft September 10, 2025 16:36
@itaybre
Copy link
Contributor Author

itaybre commented Sep 10, 2025

Looks like there are several places relying on the previous behavior, I will rename the method and create a new one then

@itaybre itaybre changed the title fix: Always execute async even if called from main thread chore: dispatchAsyncOnMainQueue to dispatchAsyncOnMainQueueIfNotMainThread Sep 11, 2025
Copy link
Contributor

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentrySubClassFinder.m

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.

_SentryDispatchQueueWrapperInternal.dispatchAsyncOnMainQueue may execute in a synchronous way
1 participant