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

Delivering notifications within write transactions makes them hard to use #4818

Closed
bdash opened this issue Apr 4, 2017 · 54 comments
Closed
Assignees

Comments

@bdash
Copy link
Contributor

bdash commented Apr 4, 2017

In v2.1.0 we started delivering collection notifications when beginning a write transaction advances the read version of a Realm. We deliver these notifications within the write transaction to prevent starvation when another thread is frequently writing to the Realm. Users have found that confusing to deal with a notification block potentially being invoked within a write transaction (#4815, #4701, #4511, and likely others). There are certain operations, such as registering for object or collection notifications, that cannot be performed within a write transaction. It's not clear how a user can ever safely use such functionality within a notification callback when it may sometimes be invoked within a write transaction (other than by never performing writes on a thread that is observing a collection or object notification?).

@tgoyne mentioned on Slack that we may be able to deal with this by separating acquisition of the write lock from beginning the write transaction. This would allow us to avoid starvation while also allowing notification callbacks to perform operations that cannot be performed within a write transaction.

@TeunVR
Copy link

TeunVR commented Apr 12, 2017

Related to this:

the way this mechanism is currently working can cause inconsistencies (crash) in for example a UITableView.
If for example two write transactions are performed in the same runloop (or different threads), the second write may trigger the notification synchronously because the realm is advanced to the latest version.
In our case we do a tableView.reloadData here.
This will cause the tableView to call numberOfRows synchronously, but schedule cellForRow-calls on the next runloop.
The second write-transaction causes the results to change (less rows in my testproject for example). This does not trigger the notification before the cellForRow-calls are performed. The tableView still expects more rows than are available in the results now.

I have created a sample app to demonstrate this. See RealmNotifications.zip

The tableview shows the results of Persons that are younger than 18.
Initially there is one person in the list.
When the crash-button is pressed another Person is added, but in two steps. First it is added with age 0, then in the next write the age is set to 20 (thereby dropping out of the query).

ViewController1: most simple approach. Just a notification-block that calls reloadData. This crashes, because the second write triggers the notification synchronously by advancing.

ViewController2: attempt to workaround this by queuing the notification on the next runloop. This seems to 'solve' the problem.

ViewController3: shows the problem with the workaround of ViewController2. There is still a small gap where there could already have been cellForRows scheduled. This still crashes.

ViewController4: always call refresh after each write-transaction, triggering the notification synchronously.

What is the best way to workaround this problem? I don't have a really good feeling with these two 'workarounds'.
The value returned in numberOfRows should always match the available rows (results) in the cellForRow.

There is a mechanism to not notify a specific token, but i do want to be notified actually.

@BrandonZacharie
Copy link

I've been struggling with this for a couple of days. I haven't been able to find a way to prevent table view crashes when applying updates that originate from a realm modified on a background thread. Are there any known workarounds?

@BrandonZacharie
Copy link

BrandonZacharie commented Jul 17, 2017

Any update on this? I think this should be labeled bug rather than enhancement and given a higher priority because the changes in notifications are unusable. Personally, I've only been able to use reloadData() with a UITableView or UICollectionView. While the examples from @TeunVR are spot on, they don't demonstrate the scenario of receiving updates triggered from changes made on a background thread; I tried applying the hacks he used to get it working to no avail.

I'm not 100% sure this issue is related to #4425 which is exactly what I'm experiencing. If not, I suppose I need to open a new ticket since that one is closed.

@fwx
Copy link

fwx commented Sep 11, 2017

Any updates on this?

@stuartro
Copy link

I'm running into this exact same problem, with an RxSwift "event pipeline" that flatMaps notifications to new Observables (using RxRealm's Observable.from(..., properties:...)). I have tried everything I know, but cannot think of any way around creating new Observables within an Observable that is called in response to Realm notifications. Any updates on this issue or solutions / workarounds?

@stuartro
Copy link

Trying to work around this gets into other issues. Using observeOn() and passing a ThreadSafeReference fails with "Cannot obtain thread safe reference during a write transaction.", since the code that creates the ThreadSafeReference is on the main thread (same thread as the Realm transaction)... Nasty bug / issue this.

@fwx
Copy link

fwx commented Oct 12, 2017

@jpsim can you please tell us what's going on about this issue?

@bdash
Copy link
Contributor Author

bdash commented Oct 12, 2017

When we have an update to share, we'll do so here.

@NikKovIos
Copy link

NikKovIos commented Feb 8, 2018

It is a priority task, isn't it?

@cyupa
Copy link

cyupa commented Feb 9, 2018

Running into the exact same issue as @stuartro

@asDButcher
Copy link

Still don't have any solutions for this issue?
Man... almost a year, I still using reloadData() instead of the animation of UITableView.

@SamiAario
Copy link

We have encountered this same issue in our project as well, and our current workaround is to write to the database from separate thread pool threads, so for us this issue is clumsy but not crippling.

It would be nice to have a proper solution within Realm itself however, and as has been pointed out, the fix would seemingly be a rather simple one of acquiring the write lock, delivering the collection notifications, and only then starting the write transaction.

@virtualrapha
Copy link

@stuartro I know this thread is quite old, but I'm running into the exact same issue with my RxSwift pipeline. Did you somehow find a workaround?

@Double-Dude
Copy link

I have a scenario that Person owns and creates a Dog when Person gets created, if a Person gets added to Realm DB then I observe Dog as I want to know the change of Dog.

So I first observe Person collection, then if a Person get inserted, I immediately create observer to observe Dog(person.dog.observe), then the app crashes with message:
"Cannot register notification blocks from within write transactions."

I think realm.beginTransaction(person.insert()) triggers the personCollection.observe block then it calls dog.observe and that's why it has the message above. How should I observe a dog of a person when a person gets inserted/updated?

@stuartro
Copy link

@raphaklr Yes, I found a solution that works for me. The issue occurred when I tried to perform updates as a result of a notification—which was fired as a result of a preceding update, while the (original) Realm write transaction was still in progress. The solution (at least, what's worked flawlessly for me) is to detect the condition and not start a new write transaction if one is already open). See my post at the link below.

#4858 (comment)

If you want a more detailed code snippet lete know and I'll see what I can do.

@whitehat007
Copy link

whitehat007 commented Aug 16, 2018

Here's what I've done that's almost completely eliminated the problem:

fileprivate var deleteAttempts: Int = 0
/// Deletes the specified conversation permanently.
///
/// - Parameters:
///   - conversationId: ID of conversation to delete
///   - completion: Optional completion with error
func deleteConversation(_ conversationId: String, completion: ((_ error: Error?) -> Void)?) {
    let url = "\(baseURL)/v2/me/conversations/\(conversationId)"
    
    deleteAttempts += 1
    oauthSession.request(.delete, url, parameters: [:]).responseData { response -> Void in
        switch response.result {
        case .success(let value):
            do {
                print(value)
                let realm = try Realm()
                let conversation = realm.objects(Conversation.self).filter("id == %@", conversationId).first!
                if !realm.isInWriteTransaction {
                    realm.beginWrite()
                    realm.delete(conversation)
                    try realm.commitWrite()
                    self.deleteAttempts = 0
                    
                    completion?(nil)
                } else if self.deleteAttempts <= 10 {
                    print("Realm in write transaction! Will retry in \(self.deleteAttempts)s...")
                    Dispatch.delay(Double(self.deleteAttempts), closure: {
                        self.deleteConversation(conversationId, completion: completion)
                    })
                } else {
                    print("Too many consecutive failures! Perhaps Realm is stuck in a write transaction?")
                    self.deleteAttempts = 0
                    completion?(RLMError.fail as? Error)
                }
            } catch(let error) {
                self.deleteAttempts = 0
                completion?(error)
            }
        case .failure(let error):
            self.deleteAttempts = 0
            completion?(error)
        }
    }
}

This retries the write transaction up to 10 times with increasing delays, allowing the current write transaction to complete. When I'm trying to perform multiple transactions at once (such as updating a list of objects), I add a small random modifier to the delay (anywhere between ±0.5 seconds) to avoid continual collisions between updates. So far, I've never seen my app need more than two attempts to complete write transactions with up to ten different objects simultaneously. This way, I can call multiple write transactions from notifications without having to worry about crashing.

@BrandonZacharie
Copy link

BrandonZacharie commented Dec 18, 2018

I first ran into trouble with notifications in July last year. Our app makes heavy use of applying updates that originate from a realm modified on a background thread and the implementation has not changed since my first comment. After upgrading to Realm 3.11.2 last month, I tried switching from reloadData() back to using change notifications and I have not seen a crash so far.

@tgoyne I think it is safe to say my issue is resolved but I don't know what changed to make that happen.

@BrandonZacharie
Copy link

I just tried the demo project from @TeunVR with Realm 3.13.0 and it still crashes. However, that specific issue can be resolved with by wrapping tableview update calls with DispatchQueue.main.async or with a realm.refresh() call after writes. I think documentation may be needed there; I suppose those new to Realm (or unfamiliar with threading) will footgun it.

@OndrejHRIC
Copy link

OndrejHRIC commented Feb 5, 2019

I had same issue after removing condition if isInWriteTransaction { return } when it crashes with reason, that thread safe reference cannot be obtained during write transaction and it was caused by unnecessary call to add object to realm, but that object was in realm already.

It has complicated logic, but in general where was some object that holds reference to taken raw pics on disk. that pics ([UIImage]) is appended to array in realm ([String]) with for cycle one by one for each image in write transaction. this appending of filenames notifies realm token and uploading starts when all images are stored as filenames. but after appending filenames I was calling add(object:, update:) because of another logic and that call updates nothing at all and cause write transaction that don't notify realm token with change. So after adding condition to make .add func called only when object isn't in realm already solves the problem.

@eliburke
Copy link

eliburke commented Mar 1, 2019

Been struggling with a notification issue myself. My app sends messages that must be saved in draft form before sending, and then updated after upload to server. The primary key changes so I do a delete+create transaction on a background thread. Both the outgoing and final messages can also be simultaneously modified from main or background threads for other processing.

When these writes are happening quickly, the size of the observed ResultSet can change while another notification is being processed and the table is being batch-updated. Because I use the value of ResultSet.count for the UITableView.numberOfSections() I end up with NSInternalInconsistencyException. (same could apply to numberOfRowsInSection()).

While sitting here dreading the thought of implementing a serial queue for database updates I figured out a workaround. I added var observedSectionCount: Int? and set its value within the .update notification. Now in numberOfSections() I always return observedSectionCount instead of accessing the "live" value from the ResultSet. Race condition appears to be solved. Hope this helps someone.

@RetVal
Copy link

RetVal commented Apr 19, 2019

It has been more than two years, any updates on this? @bdash

@shayneoneill
Copy link

Any timeline for a fix on this? Its a showstopper.

@diogoAntunes
Copy link

Please someone work on this!!

@IsaiahJTurner
Copy link

This would be awesome. I'm trying to create another observer within a notification block but that logic crashes often due to "Cannot register notification blocks from within write transactions."

@byroncoetsee
Copy link

So I took some inspiration from the above answers (starting the observer on mainThread) and made this little extension which seems to work for me...

extension Object {
    public func safeObserve(_ block: @escaping (ObjectChange<`Self`>) -> Void, token: @escaping ((NotificationToken) -> Void)) {
        DispatchQueue.main.async {
            token(self.observe { block($0) })
        }
    }
}

and usage

        someObject.safeObserve({
            [weak self] change in
            self?.someFunctionOrWhateverYouWant()
        }, token: {
            [weak self] token in
            self?.ChangeNotificationToken = token
        })

Since the actual observe call needed to happen on main thread and not just the block, the notification token that it returns would technically only be generated "later", hence it needs to live in its own closure.

Provided as is. haha. Works nice for me but can't guarantee for others 😅

@junsukim1994
Copy link

junsukim1994 commented Oct 21, 2021

In my case without having to adopt the extension above, just wrapping .observe { } code inside dispatchQueue.main.async fixed the problem for me. Still waiting for further insight into this problem...

@theoks
Copy link

theoks commented Mar 30, 2022

We are still facing several of these issues, especially when we use the built in Combine publishers, where we don't actually control the queue where the notification block is created. Are there any upcoming fixes or updates on this issue?

@andresitsu
Copy link

My app still facing this problem...it's been several years, will it be fixed?

@JuliusHuizing
Copy link

Same here: running into this bug for > 3 months now.
Tried all solutions in this issue; although they seem to have reduced the frequency with which the issue occurs, it still causes my app to crash on a regular basis.

Any updates on a structural solution?

@dianaafanador3
Copy link
Contributor

HI @JuliusHuizing Can you please explain the issue you are experience?, so we can keep track of this here or open a new issue.

@JuliusHuizing
Copy link

JuliusHuizing commented Jul 4, 2022

@dianaafanador3 Every now and then, my app crashes at run-time due to this error:

Thread 1: "Cannot register notification blocks from within write transactions."

It might very well be that I am doing something wrong myself, but having tried all the potential fixes suggested by others above, I still run into this error on a regular basis (although it seems less frequently than before).

My set-up is as follows:

  • I am using Realm Swift with Flexible Sync.
  • I have constructed a "RealmReactionService" class that monitors changes to a specific Realm collection, and performs some given action when that collection changes:
class RealmReactionService: Service, ObservableObject {

    var collectionOneToken: NotificationToken?
    var collectionOneResults: Results<CollectionOne>?
    
    var realm: Realm? { willSet {
        if let realm = newValue {
            self.collectionOneResults = realm.objects(CollectionOne.self)

        }
    }}


   func whenCollectionChanges(perform action: @escaping () -> Void) async {
       await MainActor.run {
           Self.logger.trace("(Thread: \(Thread.current) Initialising notification token for changes to Collection One.")
           self.collectionOneToken = self.collectionOneResults!.observe { (changes: RealmCollectionChange) in
               switch changes {
               case .update:
                   Self.logger.debug("Observed update to collection. Will perform provided action.")
                    action()

               default:
                   print("doing nothing")
               }
           }
       }
   }

}

The action provided to the 'whenCollectionChanges' method uses the intances of CollectionOne to compute some statistic. And that statistic is then used to update some instance of another Realm collection. E.g. in the ContentView of the SwiftApp, we perform:

// set services
            self.realmReactionService.realm = realm
            await self.realmReactionService.whenCollectionChanges {
                Task {
                    await user.update(realm: realm, propertySetter: { $0.statisticUpdated = Date.now() })) {

                }

            }

Where it then looks like the update method of the user often leads to the run-time error mentioned above.
The update method looks like this:

extension User {
    func update(realm: Realm, propertySetter: (User) -> Void) async {
        // Realm Object should always be accessed from the main thread.
        await MainActor.run {
            withAnimation {
                let thawedObject = self.thaw()
                do {
                    try realm.safeWrite {
                            propertySetter(thawedObject!)
                            realm.add(thawedObject!, update: .modified)

                    }
                } catch {
                    Self.logger.warning("Failed to update Realm Object of type \(self)")

                }
            }
        }
    }
}

And the SafeWrite is implemented as follows, inspired my multiple different suggestions done by people above:

extension Realm {
    public func safeWrite(_ block: (() throws -> Void)) throws {
        if isInWriteTransaction {
            Self.logger.debug("Skipping write trx...")
        } else {
            try write(block)
            // needed to avoid err "Cannot register notifcaiton block from within write tranasaction"
            // see: @Brandon's comment @ - https://github.com/realm/realm-swift/issues/4818
            self.refresh()
        }
    }
}

Nevertheless, as said, I still run into this run-time error every now and then.

Any suggestions are much appreciated!

@duanesteel
Copy link

It's really strange that in 5 years and all the new versions, this most basic and fundamental feature of the entire framework is still completely broken.

@reinosutisno
Copy link

It's really strange that in 5 years and all the new versions, this most basic and fundamental feature of the entire framework is still completely broken.

My thoughts exactly!

@olejnjak
Copy link

I decided to dump Realm and switch to CoreData where possible.

@JuliusHuizing
Copy link

still facing this issue.

@LilaQ
Copy link

LilaQ commented Aug 26, 2022

Wow, this has been going on for so long and nothing has been done about it?
I guess it's time to move somewhere else

@aehlke
Copy link

aehlke commented Oct 12, 2022

Besides the workarounds above, are there usage best practices / patterns that can be avoided to sidestep this issue? I'm not encountering it but this issue frightens me from shipping my work based on Realm

@zenghaojim33
Copy link

+1 looking for best practices as well...

@cookeri6
Copy link

Seeing this issue with rapid updates to a CollectionView. Any updates on a fix for this?

tgoyne added a commit to realm/realm-core that referenced this issue May 1, 2023
…change

This fixes one of the common problems resulting from
realm/realm-swift#4818, as it makes it so that adding
new callbacks from within a notification callback will _usually_ work even if
the notification was triggered by beginning a write transaction. It still will
not work if a previously invoked callback made any changes (which would be a
very strange thing to do, but possible).
tgoyne added a commit to realm/realm-core that referenced this issue May 1, 2023
…change

This fixes one of the common problems resulting from
realm/realm-swift#4818, as it makes it so that adding
new callbacks from within a notification callback will _usually_ work even if
the notification was triggered by beginning a write transaction. It still will
not work if a previously invoked callback made any changes (which would be a
very strange thing to do, but possible).
tgoyne added a commit to realm/realm-core that referenced this issue May 2, 2023
…change (#6560)

This fixes one of the common problems resulting from
realm/realm-swift#4818, as it makes it so that adding
new callbacks from within a notification callback will _usually_ work even if
the notification was triggered by beginning a write transaction. It still will
not work if a previously invoked callback made any changes (which would be a
very strange thing to do, but possible).
@tgoyne
Copy link
Member

tgoyne commented May 2, 2023

realm/realm-core#6560 makes it so that registering new notifiers inside notifications triggered by starting a write transaction will work as long as you do so before making any changes in the transaction.

@tgoyne
Copy link
Member

tgoyne commented May 3, 2023

I've spent the last few weeks digging into this and seeing what we can do to improve things. There's a few different problems that people have run into here, with different solutions.

Registering new notifiers

When the notification system was first written, it would have been very difficult to support registering notifications inside write transactions. Back then we didn't have stable keys for objects and instead stored row numbers that could change when objects were deleted, so it'd require mapping the row number at the time you called observe() back to the row number at the start of the write transaction - something which was possible, but tricky. We now do have stable identifiers, and registering a notification inside a write transaction can mostly just work. By extension, this'll mean that creating new observations inside an observation callback will work fine even if the callback happens to be called inside a write transaction.

The remaining exception is observing objects inside the same write transaction as they were created. This has the problem that object keys aren't unique until the write is committed. A naive approach to supporting this would mean that creating an object, observing it, cancelling the write, then later creating a new object in a separate write transaction would result in that new object being observed instead. This is probably solveable, but is a somewhat tricky problem and not directly related to this issue.

Performing writes inside observation callbacks

This part is more complicated to support while still making the guarantees required for observation change information to be useful. To recap, the things we currently guarantee related to change information are:

If you never perform writes on the same thread as you are observing on

  • Inside a change notification on an object or collection the change notification will always accurately tell you what has changed in the collection passed to the callback since the last time the callback was invoked.
  • Reading collections or objects outside of change notifications will always give you the exact same values as you saw in the most recent change notification for that object.
  • Reading objects other than the observed one inside a change notification may see a different value prior to the notification for that change being delivered. Refreshes happen at the Realm level of granularity while change notifications aren't batched together into a single callback, so inside a callback you may see changes which have pending notifications.
  • Writes on different threads will eventually become visible on this thread. Explicitly calling refresh() blocks until writes which were made on other threads are visible (and the appropriate notifications have been sent) unless it's called from within a notification callback, in which case it's a no-op.

If you perform writes on the same thread, outside of notifications

  • At the start of the write transaction all of the above guarantees hold, plus the additional guarantee that you are seeing the latest version of the data.
  • Inside a write transaction the only changes you will see are those you've made so far in that write transaction.
  • Between committing a write transaction and the next set of change notifications being sent you can see the changes you made in that write transaction, but nothing else. Writes made on different threads will not become visible until the next set of notifications are sent, and performing another write on the same thread will send notifications for the previous write first.

If you perform writes inside of notifications

Here we try to maintain the above guarantees, but do not entirely succeed.

  • Callbacks which happen to be invoked before the one which perform a write behave normally. Note that while change callbacks are invoked in a stable order, it is not strictly the order which observations were added.

  • If beginning the write refreshes the Realm (which can happen if another thread is making writes as well), that triggers recursive notifications. These nested notifications will as usual report the changes made since the last call to the callback. For callbacks which go before the one making the write, this means the inner notification will be just the changes after the ones already reported in the outer notification. If the callback making the write tries to write again in the inner notification an exception is thrown. The callbacks after the one making the write will get a single notification for both sets of changes.

    After the callback completes the write and returns, none of the subsequent callbacks are invoked as they no longer have any changes to report. A notification will be produced later for the write as if the write had happened outside of a notification.

  • If beginning the write doesn't refresh the Realm, the write happens as usual and things appear to be simple. However, the subsequent callbacks are invokved in an inconsistent state: they continue to report the original change information, but the observed object/collection now includes the changes from the write made in the previous callback.

  • By doing wacky things (checking realm.isInWriteTransaction, and if so making changes, calling realm.commitWrite() and then realm.beginWrite()) you can get notifications nested more than two levels deep. This is a very bad idea.

The first important thing to note is that async writes now provide a reliable way to sidestep all of this complexity. If you don't need fine-grained change information inside your write block, you can do:

let token = dog.observe(keyPaths: [\Dog.age]) { change in
    guard case let .change(dog, _) = change else { return }
    dog.realm!.writeAsync {
        dog.isPuppy = dog.age < 2
    }
}

This will work even if the notification happens to be delivered inside a write transaction. However, because the write is async the Realm may have changed between the notification and when the write happens, so the change information passed to the notification may no longer be applicable.

If you do need the fine-grained change information to be accurate inside the write transaction, it's not obvious that that's conceptually possible, even setting aside implementation difficulties. It's also not clear that anyone actually needs this, so I think maybe it's best just ignored?


Updating a UITableView

This is not really related to notifications being delivered inside write transactions, but it's come up here a few times. Here the problem is that if you only update a UITableView via notifications, in the time between a write transaction and the next notification arriving the tableview's state is out of sync with the data. If the tableview has a pending update scheduled, then this causes problems. There's a few ways to fix this.

For all of these, suppose you have the following very basic view controller:

class TableViewController: UITableViewController {
    let realm = try! Realm()
    let results = try! Realm().objects(DemoObject.self).sorted(byKeyPath: "date")
    var notificationToken: NotificationToken!

    override func viewDidLoad() {
        super.viewDidLoad()

        tableView.register(Cell.self, forCellReuseIdentifier: "cell")

        notificationToken = results.observe { (changes: RealmCollectionChange) in
            switch changes {
            case .initial:
                self.tableView.reloadData()
            case .update(_, let deletions, let insertions, let modifications):
                self.tableView.beginUpdates()
                self.tableView.insertRows(at: insertions.map { IndexPath(row: $0, section: 0) }, with: .automatic)
                self.tableView.deleteRows(at: deletions.map { IndexPath(row: $0, section: 0) }, with: .automatic)
                self.tableView.reloadRows(at: modifications.map { IndexPath(row: $0, section: 0) }, with: .automatic)
                self.tableView.endUpdates()
            case .error(let err):
                fatalError("\(err)")
            }
        }
    }

    override func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
        return results.count
    }

    override func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        let object = results[indexPath.row]
        let cell = tableView.dequeueReusableCell(withIdentifier: "cell", for: indexPath) as! Cell
        cell.textLabel?.text = object.title
        return cell
    }

    func delete(at index: Int) throws {
        try realm.write {
            realm.delete(results[index])
        }
    }
}

Option 1: Update the UITableView directly and skip the notification

func delete(at index: Int) throws {
    try realm.write(withoutNotifying: [token]) {
        realm.delete(results[index])
    }
    tableView.deleteRow(at: [IndexPath(row: index, section: 0)], with: .automatic)
}

This gives the most responsive UI as the tableview is updated immediately rather than after a few hops between threads that adds a small amount of lag to each update. The downside is that it requires the most manual updates to the view.

Option 2: Force a refresh after the write

func delete(at index: Int) throws {
    try realm.write {
        realm.delete(results[index])
    }
    realm.refresh()
}

This results in the notifications from the write being delivered immediately rather than on a future run of the run loop, so there's no window for the table view to read out of sync values. The downside is that this makes it so that things which would normally happen on a background thread (such as rerunning the query and resorting the results) will instead happen on the main thread, and that's a bad thing.

Option 3: Do the write on a background thread

func delete(at index: Int) throws {
    func delete(at index: Int) throws {
        @ThreadSafe var object = results[index]
        DispatchQueue.global().async {
            guard let object = object else { return }
            let realm = object.realm!
            try! realm.write {
                if !object.isInvalidated {
                    realm.delete(object)
                }
            }
        }
    }
}

This blocks the main thread for the least amount of time, but is often more difficult to write. Since the write isn't done on the main thread, the main thread never sees the write before the notifications arrive.

Option 4: Add an option to make writes not immediately visible on the writing thread

One thing that we could do now is to make it so that after realm.write() completes, the changes made in that write aren't visible on the current thread until the next time notifications are delivered. This'd make the whole problem go away entirely. It also seems like it could be very confusing, and it would interact weirdly with KVO (we'd have to notify of changing from the new value to the old value and then back again).

@floorish
Copy link

floorish commented May 4, 2023

Thanks for the writeup @tgoyne — would be nice to see these details in the docs. Happy with the ability to register new notifiers in a write transaction.

Question about Updating a UITableView Option 3: would using the writeAsync API also be an solution, or does that still leave a window where the calling thread could have an out of sync view of the data?

For example:

func delete(at index: Int) throws {
    realm.writeAsync({    
        realm.delete(results[index])

        // (A)
        // at this point `results` is updated, but no notification sent yet
        // => out of sync with data

    }, onComplete: { err in
        // (B)
        // notification has been fired
        // => in sync with data
    })
}

I guess it is possible that between point A and B another block gets scheduled on this thread, that sees the write before the notification is sent?

Option 4 sounds like a nice solution to solve this, but I agree, probably too confusing and complex for most use cases.

@tgoyne
Copy link
Member

tgoyne commented May 4, 2023

writeAsync wouldn't fix it. I suspect it might actually make you run into the problem more often. Moving the writing to disk off the main thread means that there's a longer period of time that you're in an inconsistent state with the main thread unblocked and able to schedule work.

@tgoyne
Copy link
Member

tgoyne commented May 8, 2023

Support for adding new observers inside notifications triggered by beginning a write was released in 10.39.1 and I believe async writes are an adequate solution to performing writes inside notifications, so the problems which this issue were originally tracking have been fixed. I created realm/realm-core#6587 and realm/realm-core#6588 to track further follow-up work on the related issues that people have mentioned here. The docs team has created a ticket to incorporate the information I wrote up about the guarantees notifications provide, but that isn't publicly visible.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests