Prevent possible race condition in Main Presenter by delaying credential publish

[martynhaigh]

This is quite unlikely but it seems to me that by using share for the credentials observable, you risk a possible race condition where signInEnable, signInDisabled and signInResult could be fed different values.

[artem-zinnatullin]

It's impossible because thread that calls Presenter.bind() is same as thread that produces UI events from View, it's Main Thread, so there is no race.

Though, yeah, 1st principle of RxUi is to hide knowledge about Main Thread from Presenter/ViewModel layer.

So, Idk, @martynhaigh after my explanation you still want to change share() to ConnectableObservable?

[artem-zinnatullin]

Oh shi, I think race is still possible in one case: when view has initial data, like after restoration from previous state, then it will emit it after first subscription and next chains won't process this initial state.

So I'm on your side then! Please fix comment about subscription and add same code to the sample app written in Kotlin (or I can do it myself if you want).

[martynhaigh]

Good catch - I'd not thought about the restore state story. I'm only just learning Kotlin but it's looks pretty easy to transition to from Java, and this fix is pretty simple so I gave it a shot - pretty sure it's good.

[artem-zinnatullin]

Ok, cool! Ping me when you'd like me to review code again!

On Mon, 6 Jun 2016, 13:17 Martyn Haigh, notifications@github.com wrote:

Good catch - I'd not thought about the restore state story. I'm only just learning Kotlin but it's looks pretty easy to transition to from Java, and this fix is pretty simple so I gave it a shot - pretty sure it's good.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/artem-zinnatullin/RxUi/pull/15#issuecomment-223920329, or mute the thread https://github.com/notifications/unsubscribe/AA7B3GahOa61junl_tVXT6cSTG3N2VKkks5qI_OngaJpZM4IuHzK .

[martynhaigh]

Did the PR not update? I'm seeing the new code (sha e4bf4e2) ready for review just above my previous comment

[artem-zinnatullin]

Oh, sorry, was replying from email client and didn't relealize that you've already pushed new code, my bad…

👍

[artem-zinnatullin]

Thanks a lot, Martyn!