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

Wrap write transactions in background tasks on supported platforms #4827

Closed
wants to merge 1 commit into from

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented Apr 6, 2017

This asks the OS to not suspend the app while a write is in progress, which can help prevent apps which share realm files between processes from getting "stuck" due to a suspended app holding the write lock. Because unconditionally wrapping things in tasks is extremely slow, it uses the app state change notification to only do so when the app is actually running in the background.

The manual testing I did for this (since it isn't really possible to write tests for) was:

  1. Run performance tests
  2. Run repro case from Deadlock race condition using realm within app group #4797 and verify that everything works as expected (on device only the active app makes forward progress, but I could never get the active app to get "stuck" by rapidly switching between the apps and/or killing them)
  3. Run extension example on device and verify that still works.

Fixes #4797.

@tgoyne tgoyne self-assigned this Apr 6, 2017
@tgoyne tgoyne requested a review from jpsim April 6, 2017 19:13
}

- (void)applicationDidEnterBackground:(NSNotification *)notification {
if (notification && !self.inWriteTransaction) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this notification potentially be delivered on a different thread than the one this RLMRealm is associated with? I don't think it's safe to call -[RLMRealm inWriteTransaction] on a Realm from a different thread.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I think it'll always be called on the main thread. inWriteTransaction doesn't check the thread so it'll usually work by coincidence, but it's a race condition.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're using this approach and actually it's easy to receive Realm accessed from incorrect thread exception when trying to cancel a transaction in the write state.

This asks the OS to not suspend the app while a write is in progress,
which can help prevent apps which share realm files between processes
from getting "stuck" due to a suspended app holding the write lock.
@pauluhn
Copy link

pauluhn commented Apr 18, 2017

2.6.0?

@jpsim
Copy link
Contributor

jpsim commented Apr 18, 2017

No, this didn't make it into 2.6.0 because of the concern raised in https://github.com/realm/realm-cocoa/pull/4827/files#r110246828.

@egoldfarb
Copy link

egoldfarb commented Jul 13, 2017

I believe NSFileCoordinator is a better approach. See my comment:
#4797 (comment)

@tgoyne tgoyne marked this pull request as draft November 30, 2021 16:27
@tgoyne
Copy link
Member Author

tgoyne commented Aug 16, 2024

  • [ ]

@tgoyne tgoyne closed this Aug 16, 2024
@tgoyne tgoyne deleted the tg/background-task branch August 16, 2024 18:59
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deadlock race condition using realm within app group
7 participants