Codecov Report

Merging #857 (1b8333a) into master (2aa321b) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #857      +/-   ##
==========================================
+ Coverage   82.57%   82.59%   +0.02%     
==========================================
  Files          53       53              
  Lines        7368     7372       +4     
  Branches     1186     1187       +1     
==========================================
+ Hits         6084     6089       +5     
+ Misses       1175     1174       -1     
  Partials      109      109              

Nice catch, thanks for getting this fixed. Can you estimate how severe this issue is for mobile SDKs? If this has a bigger impact I'm happy to help you getting this shipped for all relevant mobile SDKs.

It is hard to say, but I don't think this will affect many.

The options lock is a recursive mutex so you can go pretty wild with nested locking solely on options. Also, the options lock usually is short-lived because it only guards the access to the ref-count to retrieve a read-only reference. The nesting of options -> scope -> options is generally a lousy pattern if one thread acquires the scope lock before another locks options at the start of sentry_init() 😓 .

How realistic this is on the Mobile SDKs depends on how they interact with the Native SDK (threads already running that could access scope-related functions before another thread calls sentry_init(), for instance... I think we talked about this recently). I don't know if the last ANR was a rare case or if there is an uptick, like in our CI.

In any case, we can release it immediately, but I am unsure if it is urgent.

The other aspect is that with the current change, you must ensure that the global scope hasn't been touched before sentry_init() or the initialization defaults to the build-time SDK name.

We can change this by not setting the dynamic SDK name in get_scope() at all but rather doing it in the SENTRY_WITH_SCOPE_MUT in sentry_init(). This way, we could eliminate the options path towards get_scope() and an optionally set sdk_name will always overwrite the scope in sentry_init(). WDYT?

We can change this by not setting the dynamic SDK name in get_scope() at all but rather doing it in the SENTRY_WITH_SCOPE_MUT in sentry_init(). This way, we could eliminate the options path towards get_scope() and an optionally set sdk_name will always overwrite the scope in sentry_init(). WDYT?

Just in case it is unclear: this would mean we would have to keep the client_sdk unfrozen in get_scope() and freeze it in sentry_init() together with setting the sdk_name. It boils down to these two options (they are two sides of a coin).

Superseded by https://github.com/getsentry/sentry-native/pull/858/files