Fix WithLatestFrom potential memory visibility issues

[akarnokd]

As I reported in #227, setting a volatile hasLatest doesn't ensure proper visibility of the latest value and one needs at least an ordered store for the whole element (by boxing it and writing the reference in with an ordered store).

Cost of correctness: one boxing and one unboxing of structure types.

Possible tradeoff choice: using Volatile.Write instead of Interlocked.Exchange; both are correct but the former may only become visible a bit delayed (once the CPU write buffer is emitted) whereas the latter is "immediately" visible to other cores.

[dnfclas]

Hi @akarnokd, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, DNFBOT;

[dnfclas]

@akarnokd, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

[akarnokd]

=== TEST EXECUTION SUMMARY ===
   Tests.System.Reactive  Total: 3419, Errors: 0, Failed: 0, Skipped: 2, Time: 20.187s

Tests passed but then something failed.

[dnfclas]

Hi @akarnokd, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

[bartdesmet]

Please rework to avoid the boxing. I'd prefer a plain old lock over boxing with arbitrary box lifetimes which solely depend on the incoming sequence's timing.

[akarnokd]

@bartdesmet That may only be possible if I use locks; I don't know the cost of boxing but I'd assume its less than taking a lock.

[bartdesmet]

@akarnokd - I'm not as much concerned about the cost of boxing than I am about the effects down the line. In the Bing service I'm working on now, we're very allergic to any boxing without prior knowledge about the eventual lifetime of the box: if it's known to be short-lived, it's ok (if the box can't be avoided at all); if it's known to be very long-lived (ideally infinitely long), it's ok to let it age to Gen-2 (if the box can't be avoided at all); if it's somewhere in the middle, we'll pay the cost during a GC down the line. In this particular case, we're dealing with lifetimes dependent on the velocity of the TSecond stream and the struct may be arbitrarily big (think the new ValueTuple<...> for C# 7.0 as a great source of N-ary composite objects).

My recommendation would be to use a lock (correctness above everything else), as we do in many places, and keep the costs in the control plane (IDisposable, cf. your other PR, thanks!) and on the input side (subjects) low by exploring lock free approaches. For high-performance scenarios, the best advice I'd have is to have many instances of single-threaded Rx executors with operators that don't do any locking at all. That's closer to what we do in Bing with single-threaded schedulers (N of which run in a process, where N is aligned with the number of cores) that merge events from many sources (a la what Tx does) onto a single event dispatch loop. It makes an assumption though about being able to control the threading and where the sources produce their messages (easier to do in a service where all the input comes from big pipes over the network). This would also work well for UI frameworks etc. that are single-threaded anyway.

[akarnokd]

Okay. I'll change it to use a lock.

[bartdesmet]

Cool, thanks! Do you still have issues with contribution agreement we have to look into?

[akarnokd]

No, found another link and signed it (see cla-already-signed label).

[akarnokd]

Updated the PR, not sure what takes so long for the CI (I get access denied is it may not even run yet).

[bartdesmet]

Can you change the field name back to _hasLatest for consistency and to minimize the diff?

[akarnokd]

Done.

[shiftkey]

👍

[clairernovotny]

@shiftkey @bartdesmet @mattpodwysocki is this good to merge?