feat: Validate single occurence of Sentry SDK inbinaries #5298

itaybre · 2025-05-26T21:24:49Z

📜 Description

Add a log message when Sentry SDK is found to be present twice + Added a breakpoint

💡 Motivation and Context

Fixes #4566

💚 How did you test it?

Running duplicated SDK app + Unit Tests cases

📝 Checklist

You have to check all boxes before merging:

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.
I updated the wizard 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.

TODO:

See how to tests this without needing to capture app output
Define how we want to raise this error

github-actions · 2025-05-26T21:25:14Z

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 62a7835

codecov · 2025-05-26T21:29:08Z

Codecov Report

Attention: Patch coverage is 98.63014% with 1 line in your changes missing coverage. Please review.

Project coverage is 86.463%. Comparing base (94a6b1a) to head (62a7835).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
Sources/Swift/Tools/LoadValidator.swift	98.412%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #5298       +/-   ##
=============================================
- Coverage   86.671%   86.463%   -0.209%     
=============================================
  Files          419       419               
  Lines        35864     35769       -95     
  Branches     16923     15175     -1748     
=============================================
- Hits         31084     30927      -157     
- Misses        4498      4802      +304     
+ Partials       282        40      -242

Files with missing lines	Coverage Δ
Sources/Sentry/SentryBinaryImageCache.m	`100.000% <100.000%> (ø)`
Sources/Sentry/SentrySDK.m	`88.028% <100.000%> (ø)`
Sources/Swift/Tools/LoadValidator.swift	`98.412% <98.412%> (ø)`

... and 58 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 94a6b1a...62a7835. Read the comment docs.

philprime

Thanks @itaybre for working on this! Left a couple of comments.

Sources/Swift/Core/Tools/LoadValidator.swift

Tests/DuplicatedSDKTest/Unit Tests/Unit_Tests.swift

Sources/Swift/Core/Tools/LoadValidator.swift

Tests/DuplicatedSDKTest/Unit Tests/Unit_Tests.swift

Tests/DuplicatedSDKTest/DuplicatedSDKTest.xctestplan

Sources/Swift/Core/Tools/LoadValidator.swift

philipphofmann

Thanks for tackling this.

Sources/Swift/Core/Tools/LoadValidator.swift

philprime · 2025-06-23T07:37:37Z

I believe we should update the PR branch before continuing review

armcknight

I ran the tests with a breakpoint in here, and it tripped on it in:

SentrySDKTests
SentryTests
SentryStackTraceBuilderTests

And then I stopped because it got annoying to exhaustively check 🙃

I noticed that it would still execute even if there was no test function on the main thread (or really anything at all) or when cxx_destruct was on the main thread (where it would take the guarded early return on line 45)

This is why I called out maybe using SentryDispatchQueueWrapper, just in case this could ever affect state in other tests.

The gist of it looks fine, just a couple suggestions for you.

Sentry.xcodeproj/project.pbxproj

Tests/DuplicatedSDKTest/Unit Tests/Unit_Tests.swift

Sources/Swift/Tools/LoadValidator.swift

philipphofmann

This looks pretty great. Thanks for tackling this @itaybre. I'm sorry I found a pretty long list for things to improve.

Do you know how big the overhead of running this is, @itaybre? I'm a bit afraid that this could slow down the app start for users using debug. We already have complaints about the other debug image cache we're using here #4618

Tests/DuplicatedSDKTest/Unit Tests/Unit_Tests.swift

Sources/Sentry/Public/SentryBinaryImageInfo.h

Sources/Sentry/SentryBinaryImageCache.m

Sources/Swift/Tools/LoadValidator.swift

philipphofmann

Thanks for including all the feedback. We're getting close to LGTM.

Sources/Sentry/include/HybridPublic/SentryBinaryImageInfo.h

Sources/Swift/Tools/LoadValidator.swift

Tests/SentryTests/Swift/Core/Tools/LoadValidatorTests.swift

Sources/Swift/Tools/LoadValidator.swift

Tests/SentryTests/Swift/Core/Tools/LoadValidatorTests.swift

sdk_api.json

…rameters.

…r/CoreSimulator/Volumes`

philipphofmann

Thanks @itaybre, LGTM

Sources/Sentry/SentryBinaryImageCache.m

Sources/Swift/Tools/LoadValidator.swift

…d LoadValidator

github-actions · 2025-07-22T17:00:00Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1236.85 ms	1256.79 ms	19.94 ms
Size	23.75 KiB	904.53 KiB	880.78 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`d7461dc`	1233.69 ms	1255.29 ms	21.60 ms
`4e3915a`	1230.02 ms	1258.90 ms	28.88 ms
`5258fb8`	1207.92 ms	1234.51 ms	26.59 ms
`cc7f629`	1226.00 ms	1245.51 ms	19.51 ms
`a9fac2e`	1212.45 ms	1219.67 ms	7.22 ms
`0529194`	1237.23 ms	1254.67 ms	17.44 ms
`30f4e4c`	1213.21 ms	1250.29 ms	37.08 ms
`354b020`	1223.88 ms	1236.82 ms	12.94 ms
`ae7be93`	1236.24 ms	1258.18 ms	21.94 ms
`6279992`	1213.60 ms	1241.38 ms	27.79 ms

App size

Revision	Plain	With Sentry	Diff
`d7461dc`	23.75 KiB	874.45 KiB	850.70 KiB
`4e3915a`	23.75 KiB	858.69 KiB	834.94 KiB
`5258fb8`	23.75 KiB	874.45 KiB	850.70 KiB
`cc7f629`	23.75 KiB	878.48 KiB	854.73 KiB
`a9fac2e`	23.75 KiB	879.53 KiB	855.78 KiB
`0529194`	23.74 KiB	891.02 KiB	867.28 KiB
`30f4e4c`	23.75 KiB	879.24 KiB	855.50 KiB
`354b020`	23.75 KiB	878.19 KiB	854.44 KiB
`ae7be93`	23.75 KiB	879.24 KiB	855.49 KiB
`6279992`	23.75 KiB	891.03 KiB	867.28 KiB

itaybre changed the title ~~feat: Add class loaded twice validator~~ feat: Validate single occurence of Sentry SDK inbinaries May 27, 2025

itaybre marked this pull request as ready for review May 27, 2025 14:25

itaybre requested review from philipphofmann, armcknight, philprime, noahsmartin, trevor-e and NicoHinderling as code owners May 27, 2025 14:25

philprime reviewed May 28, 2025

View reviewed changes

noahsmartin reviewed May 31, 2025

View reviewed changes

Sources/Swift/Core/Tools/LoadValidator.swift Outdated Show resolved Hide resolved

noahsmartin reviewed May 31, 2025

View reviewed changes

Sources/Swift/Core/Tools/LoadValidator.swift Outdated Show resolved Hide resolved

philipphofmann reviewed Jun 18, 2025

View reviewed changes

itaybre force-pushed the itaybre/feature/duplicated_class_log branch from 299a80a to c442ab3 Compare June 23, 2025 16:13

itaybre marked this pull request as draft June 23, 2025 20:41

itaybre force-pushed the itaybre/feature/duplicated_class_log branch from ea33ac5 to d56311b Compare July 15, 2025 18:34

itaybre marked this pull request as ready for review July 15, 2025 18:37

armcknight approved these changes Jul 17, 2025

View reviewed changes

philipphofmann reviewed Jul 17, 2025

View reviewed changes

philipphofmann reviewed Jul 18, 2025

View reviewed changes

itaybre mentioned this pull request Jul 21, 2025

ref: Remove SentryDefaultObjCRuntimeWrapper singleton pattern and use SentryDependencyContainer #5670

Merged

itaybre and others added 7 commits July 21, 2025 15:57

feat: Add class loaded twice validator

9ee3d32

Add basic test capturing print output

9711cc0

Update changelog

be079b1

Update min target version

adb2781

Remove headers

1cf9444

Update changelog

a32f36b

Add basic test capturing print output

b7b5028

itaybre added 20 commits July 21, 2025 15:57

Move SentryBinaryImageInfo to HybridPublic

8525972

Add isDebug paramter to SentryBinaryImageCache

f518833

Update broken tests

14845ce

Avoid nesting

8a15d9d

Fix test

824b8bb

Fix SentryBinaryImageInfo not available for swift by receiving the pa…

ecb266c

…rameters.

Add comment referencing SentrySubClassFinder

6ca1401

Log an error instead of a warning

cd22e23

Fix unit test

d93af26

Set debug value before registering the callback

bc00b81

Update stability jsons

fdba521

Remove import

6941b12

Remove unnecesarry impot

75e0130

Revert extracting SentryBinaryImageInfo to a new file

79b92b1

Revert extracting SentryBinaryImageInfo to a new file

7234ba6

Use hasPrefix and update simulator ignored path to `/Library/Develope…

063959a

…r/CoreSimulator/Volumes`

Fix typo

a61df55

Update tests

f7f9eb8

Fix support for Xcode 14.0

f740109

Pass dispatchQueueWrapper as an argument

c0ab737

itaybre force-pushed the itaybre/feature/duplicated_class_log branch from e473900 to c0ab737 Compare July 21, 2025 19:21

itaybre added 2 commits July 21, 2025 16:40

Update CHANGELOG and modify imports in SentryPrivate.h

47958f9

Fix target module for SomeWork class

6379a7a

getsentry deleted a comment from github-actions bot Jul 22, 2025

philipphofmann approved these changes Jul 22, 2025

View reviewed changes

Sources/Sentry/SentryBinaryImageCache.m Outdated Show resolved Hide resolved

Sources/Swift/Tools/LoadValidator.swift Show resolved Hide resolved

Sources/Swift/Tools/LoadValidator.swift Show resolved Hide resolved

itaybre added 2 commits July 22, 2025 10:28

Add comments to clarify validation logic in SentryBinaryImageCache an…

b6d2b2d

…d LoadValidator

Bump diffMax to 885kb

62a7835

itaybre merged commit 9e6569a into main Jul 22, 2025
130 of 135 checks passed

itaybre deleted the itaybre/feature/duplicated_class_log branch July 22, 2025 17:14

Uh oh!

feat: Validate single occurence of Sentry SDK inbinaries #5298

feat: Validate single occurence of Sentry SDK inbinaries #5298

Uh oh!

Conversation

itaybre commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

TODO:

Uh oh!

github-actions bot commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

philprime left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philipphofmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philprime commented Jun 23, 2025

Uh oh!

armcknight left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philipphofmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philipphofmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philipphofmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

itaybre commented May 26, 2025 •

edited

Loading

github-actions bot commented May 26, 2025 •

edited

Loading

codecov bot commented May 26, 2025 •

edited

Loading