Write/Commit Async API #2899
Write/Commit Async API #2899
Conversation
There was a problem hiding this comment.
Reviewed the implementation, but haven't looked at the tests or docs. @fealebenpae can you double check the native code to make sure we haven't missed something.
wrappers/src/shared_realm_cs.cpp
Outdated
| return handle_errors(ex, [&]() { | ||
| return realm->async_begin_transaction([tcs_ptr, ex]() | ||
| { | ||
| s_handle_write_commit_async(tcs_ptr, ex); |
There was a problem hiding this comment.
We should not be reusing ex here.
There was a problem hiding this comment.
We can't pass ex in because it's a reference to a stack-allocated variable in C#, and the callback to async_begin_transaction can be called some time in the future (like if the write lock is held by another thread or process) when the variable no longer exists in C# and it'll crash if we try to access it. If async_begin_transaction doesn't pass in an std::exception_ptr to its callback like async_commit_transaction then we simply don't need the NativeException argument to s_handle_write_commit_async. If you need to reuse the callback type then just initialize a dummy NativeException with NoError.
wrappers/src/shared_realm_cs.cpp
Outdated
| return handle_errors(ex, [&]() { | ||
| return realm->async_begin_transaction([tcs_ptr, ex]() | ||
| { | ||
| s_handle_write_commit_async(tcs_ptr, ex); |
There was a problem hiding this comment.
We can't pass ex in because it's a reference to a stack-allocated variable in C#, and the callback to async_begin_transaction can be called some time in the future (like if the write lock is held by another thread or process) when the variable no longer exists in C# and it'll crash if we try to access it. If async_begin_transaction doesn't pass in an std::exception_ptr to its callback like async_commit_transaction then we simply don't need the NativeException argument to s_handle_write_commit_async. If you need to reuse the callback type then just initialize a dummy NativeException with NoError.
…nt. Now all planned tests are implemented
Description
This PR adds Write/Commit Async API.
The only thing that testing in this PR doesn't do is checking if cancelling while waiting for an async transaction does indeed cancel the queued callback in CORE. I don't see an easy way of testing this with the current implementation. My expectation is that this aspect is going to be easier to test when we introduce
CancellationTokens in async calls. There will be a PR for this coming soon ™️.TODO