Assertion failure: m_state == SyncUser::State::LoggedIn in sync_user.cpp:315

[nirinchev]

SDK and version

SDK : .NET
Core Version: 12.1.0

Observations

• How frequent do the crash occur?
  • Very rarely
• Does it happen in production or during dev/test?
  • Saw it on CI
• Can the crash be reproduced by you?
  • Haven't tried

Crash log / stacktrace

/Users/runner/work/realm-dotnet/realm-dotnet/wrappers/realm-core/src/realm/object-store/sync/sync_user.cpp:315: [realm-core-12.1.0] Assertion failed: m_state == SyncUser::State::LoggedIn
0   librealm-wrappers.dylib             0x0000000105a96c91 _ZN5realm4util9terminateEPKcS2_lOSt16initializer_listINS0_9PrintableEE + 369
1   librealm-wrappers.dylib             0x00000001057a24ca _ZN5realm8SyncUser17update_identitiesENSt3__16vectorINS_16SyncUserIdentityENS1_9allocatorIS3_EEEE + 186
2   librealm-wrappers.dylib             0x0000000105763011 _ZN5realm4util14UniqueFunctionIFvRKNS_3app8ResponseEEE12SpecificImplIZNS2_3App11get_profileERKNSt3__110shared_ptrINS_8SyncUserEEEONS1_IFvSF_NS0_8OptionalINS2_8AppErrorEEEEEEE3$_2E4callES5_ + 1777
3   librealm-wrappers.dylib             0x00000001057668b5 _ZN5realm4util14UniqueFunctionIFvRKNS_3app8ResponseEEE12SpecificImplIZNS2_3App24do_authenticated_requestEONS2_7RequestERKNSt3__110shared_ptrINS_8SyncUserEEEOS7_E3$_9E4callES5_ + 149
4   librealm-wrappers.dylib             0x000000010566f4e2 realm_http_transport_respond + 1202

Steps & Code to Reproduce

Looking at the code in the stacktrace, I don't believe this assertion is correct or at least the code preceding the call to User::update_identities is correct.

When App:log_in_with_credentials is called, we obtain a user that is in LoggedIn state. However, by the time the request in App::get_profile completes, there's no guarantee that the user hasn't logged out/been removed. We do call User::set_state(SyncUser::State::LoggedIn) but that's after we call update_identities:

realm-core/src/realm/object-store/sync/app.cpp

Lines 487 to 489 in 51a64fe

	sync_user->update_identities(identities);
	sync_user->update_user_profile(SyncUserProfile(get<BsonDocument>(profile_json, "data")));
	sync_user->set_state(SyncUser::State::LoggedIn);

Since we're not holding a mutex on the user while manipulating their identities and state, I don't think these assertions make sense. A logout call can race with any of these operations, resulting in an assertion failure. Instead I'd suggest that we either take a lock on the user while updating their information (and move the set_state call first) or we replace the assertions with if-checks that make the method a no-op for logged out users.

[nirinchev]

For some extra context, I don't believe it's particularly easy to trigger that crash - one really has to try or be very unlucky. So it's not the highest priority in the world, but probably a good idea to audit the relevant code and make sure it's free of race conditions.

[nirinchev]

Can we prioritize fixing this or is there a workaround we can apply for our tests? We're seeing this show up more and more often on CI. I don't believe we've made changes to C# code exercising this codepath, so not sure why it's happening more often.

[sync-by-unito]

➤ Jonathan Reams commented:

Sure, we can think of a workaround. This was put on the backlog because it was going to be resolved by a larger project, but if this is happening more frequently then we can come up with a work-around or fix it in the short-term. How frequently is this happening - once a week or once a day?

[sync-by-unito]

➤ nirinchev commented:

It's happening on pretty much all of our CI runs when we merge to main. There we test on 6 platforms and at least one will always fail, sometimes more. So I'd say 15-20% of the time.

[sync-by-unito]

➤ Jonathan Reams commented:

Okay, [~[email protected]] will take a look. I've bumped up the priority in the meantime.