-
-
Notifications
You must be signed in to change notification settings - Fork 96
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
ReplaySubject
race condition
#228
Comments
Thanks for the details, this issue is important and I will check it out right away. |
Just to confirm we are also seeing what sounds like the same bug. It looks like a really short window of race only on the first OnNext/Value call. We have a test case with just ReactiveProperty -> Select -> Switch. In production we only see the bug around 1 in 200. In our case, the Select constructs a new ReactiveProperty, subscribes over the network and calls OnNext within ~2ms. If we add a 10ms delay before the OnNext, it solves the problem, but then we need to add more locks to avoid out-of-order delivery. If we swap the Select for SelectAwait before the Switch the bug goes away. |
Seems like race condition also, because OnNext is not synchronized with subscriptions. Current value of ReactiveProperty is emitted to a new observer without a lock, and only subscription is under the lock. It is possible to call OnNext, which doesn't seem to have any synchronization at all, in between these lines and have a notification missing. |
I'd rather reimplement ReactiveProperty with locking in OnNext and SubscribeCore and see if it goes away. I think the problem is in ReactiveProperty and not in Select/SelectAwait or Switch. |
Just investigated |
Thank you, once we release the current fix code that includes the ReplaySubject issue, we will look into this issue as soon as possible. |
Fixed [ReactiveProperty race condition](Cysharp#228 (comment))
Fixed [ReactiveProperty race condition](Cysharp#228 (comment))
I've created a pull request for the ReactiveProperty issue |
I've spotted a bug with ReplaySubject, which may leave newly-subscribed observer without some new notifications in rare cases in a current version (v. 1.1.12)
When an observer subscribes to
ReplaySubject
, it receives stored notifications under a lock, but then it is registered to ReplaySubject without the lock. If in between these lines, OnNext is called with a new notification from another thread, the observer will miss the new notification because its emission is not synchronized with new subscriptions.dotnet/reactive implements
ReactiveSubject
with synchronization of both notification emission and subscription.Easy way to fix that is to put subscription (SubscribeCore) and notification emission (OnNext) under the lock too.
The text was updated successfully, but these errors were encountered: