Swap is broken

[kayceesrk]

swap is racy. In all_messages (MSQueue.snapshot e.incoming) >+> Reagent.send push_message [1], channel state can change between checking for pending offers in all_messages and posting a new message.

[1] https://github.com/theolaurent/ocaml-reagent/blob/master/communication/Channel.ml#L49

[theolaurent]

I have a simple workaround. we could have an ensure reagent : 'a casref -> ('a -> bool) -> ('b, 'b) reagent that is basically a noop except that:

• It reads the reference and call the function (which is supposed to be pure) on its content
• If the function returns false, it retries
• It also add a CAS that does not change the value on the reference, ensuring that indeed the value didn't change during the reaction built.

While deeply changing the internals might still be a good idea, because it might simplify some things, I think this is an interesting reagent anyway. And it could be use as a fix for the channels.

[theolaurent]

Or why not a "read and freeze" reagent (of type 'a casref -> (unit, 'a) reagent)? That one would read a reference while adding the same CAS that don't change the value to the reaction.

[kayceesrk]

I like it. :+1: It sounds very similar to STM read log behavior. Essentially, you read a transaction variable and commit only if the variable hasn't changed.

The interesting question is what happens when there is non-detectable changes. Support there is a reference r which happens to have a value v. With read and freeze, you'd add a cas r v v to the reaction. Concurrently, if two different reagents concurrently changes r from v --> x and then from x --> v again, should we care about the fact that value changed? Is this change benign?

STMs typically consider this change to be malign; they'd use a version number to track changes to the reference. Thoughts?

[theolaurent]

As the whole kCAS system on which are built the reactions (an thus the reagent) does not take that kind of change into account anyway, I would tend to think we can consider them as benign...