Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid blocking in Transaction::close() when there's a cancelled async write when possible #6413

Closed
wants to merge 1 commit into from

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented Mar 23, 2023

We can't cancel the wait for the write lock if another process currently holds it, but if the DB for the current Transaction holds it and we're just waiting in the write queue then we can safely close the Transaction immediately and simply skip over it when it would get its turn to write.

This used to require writing some pretty convoluted code to run into, but with structured concurrency it's much easier to accidentally write something that'll deadlock. For example, the following code does:

let realm = try Realm()
realm.beginWrite()
let task = Task {
    let realm = try Realm()
    try await realm.write {
        ...
    }
}
task.cancel()
try await task.value

@tgoyne tgoyne self-assigned this Mar 23, 2023
@cla-bot cla-bot bot added the cla: yes label Mar 23, 2023
@tgoyne tgoyne force-pushed the tg/async-write-cancel branch 2 times, most recently from ef2145d to 983dc6f Compare March 28, 2023 15:46
@tgoyne tgoyne marked this pull request as draft March 28, 2023 16:43
@tgoyne tgoyne force-pushed the tg/async-write-cancel branch 2 times, most recently from e38d7e8 to 0985a9d Compare March 29, 2023 16:35
… write when possible

We can't cancel the wait for the write lock if another process currently holds
it, but if the DB for the current Transaction holds it and we're just waiting
in the write queue then we can safely close the Transaction immediately and
simply skip over it when it would get its turn to write.
@tgoyne tgoyne force-pushed the tg/async-write-cancel branch from 0985a9d to 7e1ba33 Compare March 30, 2023 15:58
@tgoyne tgoyne marked this pull request as ready for review March 31, 2023 04:31
@tgoyne tgoyne requested review from jedelbo and ironage March 31, 2023 04:31
@jedelbo
Copy link
Contributor

jedelbo commented Apr 3, 2023

When I run realm-object-store-tests "SharedRealm: async writes" the test sometimes fails with

terminate called after throwing an instance of 'std::bad_weak_ptr'
  what():  bad_weak_ptr

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
realm-object-store-tests is a Catch2 v3.0.1 host application.
Run with -? for options

-------------------------------------------------------------------------------
SharedRealm: async writes
  close()
  before async_begin_transaction() callback
-------------------------------------------------------------------------------

and sometimes with

terminate called after throwing an instance of 'std::runtime_error'
  what():  timed_sleeping_wait_for exceeded 30000 ms

when in "while another DB instance holds lock"

@tgoyne
Copy link
Member Author

tgoyne commented Apr 4, 2023

I can reproduce both of those by enabling the sleep in DB::AsyncCommitHelper::main().

@tgoyne
Copy link
Member Author

tgoyne commented Apr 10, 2023

Closing in favor of #6486.

@tgoyne tgoyne closed this Apr 10, 2023
@tgoyne tgoyne deleted the tg/async-write-cancel branch April 10, 2023 19:15
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants