Behavior subject time gap fix 2 #1185
Conversation
|
RxJava-pull-requests #1095 FAILURE |
|
Wow this is quite the change :-) I need fresher eyes on this one so will come back to it. Thank you very much for tackling this! |
| return new PublishSubject<T>(onSubscribe, subscriptionManager, lastNotification); | ||
| @Override | ||
| public void call(SubjectObserver<T> o) { | ||
| o.emitFirst(state.get()); |
There was a problem hiding this comment.
Why is PublishSubject emitting the last received value onAdd? A PublishSubject should only receive values going forward, nothing historical.
There was a problem hiding this comment.
@akarnokd Can you help me understand this one?
There was a problem hiding this comment.
This helps eliminate race and sequence gap between adding a subscriber and terminating concurrently without the use of CountDownLatch.
There was a problem hiding this comment.
At least for BehaviorSubject. I can't remember why I left it there; maybe it is completely unnecessary and onAdded may be no-op. Either the add succeeds and the subject becomes reachable by onCompleted or fails to add and onTerminated is invoked. In the first case, state-value is null and first won't emit anything. I'll run some tests with nop tomorrow.
|
This is non-trivial code, so I may have missed something, but this looks like excellent forward progress on what we had to add the functionality needed for Very nice work. |
Behavior subject time gap fix 2
Based on discussions in #1181, I've unified the behavior among the Subjects.
BehaviorSubject doesn't seem to exhibit issue #658 and #1184.
A second pair of eyes would be great to confirm the correctness and check if the removal of the CountDownLatch was the correct approach.