RDART-1020: Fix writeAsync behaviour

[nielsenko]

Calling Realm.writeAsync with an async callback is currently broken, since the callback is not awaited, and not passed as a FutureOr<T> Function().

The first commit adds tests thats illustrates a number of issues with the current approach, and the second fixes them.

We could consider detecting re-entrancy, and either throw an exception, or simply allow it by participating in the calling transaction, but this is not handled by the current PR.

Fixes: #1667

[nirinchev]

I don't think we should do that. We're intentionally guiding people toward using the sync API because holding the write lock for extended periods of time is an antipattern and can result in app freezes in multithreaded/process scenarios and can usually be avoided by doing the async calls before entering the write transaction.

My suggestion would be to instead check if T is a future and log a warning or even error out.

[nielsenko]

I don't think we should do that. We're intentionally guiding people toward using the sync API because holding the write lock for extended periods of time is an antipattern and can result in app freezes in multithreaded/process scenarios and can usually be avoided by doing the async calls before entering the write transaction.

My suggestion would be to instead check if T is a future and log a warning or even error out.

I disagree.

We already expose beginWriteTransaction and friends, so users have all the foot-guns they need available already. writeAsync is just a 10-line wrapper around these.

If we force people to not use async callbacks in writeAsync they will just start using those APIs directly. I think it is much better to make writeAsync behave sane, and just keep warning against long running transactions. We can definitely ensure that writeAsync never blocks, as long as the callback don't.

We should however make such a check in write. Today you can pass an async callback unhindered to write, which affords no sane semantics.

[nirinchev]

Yes, I know it's not difficult to do it and I don't think we should be super actively preventing people from doing it, but I want to make it harder for them to shoot themselves in the foot. The writeAsync API is the default one people are likely to use, so if that has a future as its argument, it'll imply that we condone or even encourage doing async work in the callback. Instead, my preference would be to throw in case you pass in a future and if someone complains, explain they could achieve the same with the begin/commit API. Happy to bounce ideas about it at some point this week though as it's mostly a cautious preference rather than a strong conviction against it.