Review concepts of "document has changed", avoid race conditions

[gterzian]

In the light of the dicussion at https://github.com/automerge/spanreed/pull/21#issuecomment-1720880389

We may want to review the use of the last_heads concept inside note_changes(see current WIP), because it can lead to races conditions between saving and sending out sync messages or waking up change observers(see process_changed_documents.

[issackelly]

A note on separating out the types of reads and writes:

One of the major reasons that we're migrating to incremental saves + BEC syncing is to fix deadlock conditions in our stack. I think it would be safer to do a try_read() lock and try_write() type locks and fail with errors. In situations where it can be delayed, deferred or recoverable, we should do that.

[gterzian]

I agree about the need to prevent deadlocks.

In this particular issue, the problem was rather a livelock, in the sense that something that was expected to happen(sending out sync messages), did not(and it was not caused by an underlying deadlock). So it was rather the logic of the code that was wrong and produced a livelock.

I don't think we should switch to using try_ methods on the lock acquisitions inside repo.rs, because I don't think those can produce deadlocks(minimal scoping, no other locks being acquired in scope). Switching to "trying to acquire locks and perform certain operations" would also imply adding logic to "retry" if the lock cannot be acquired, which would add complication.

There is one potential source of deadlock I have just noted on the doc handle side of things, and it's tracked at https://github.com/automerge/spanreed/issues/53.

[issackelly]

Thank you! I appreciate the context