Unexpected UI context behaviour when calling commitAndWait() #53

New Issue

adam · 2025-12-29T15:23:08+01:00

adam commented

2025-12-29 15:23:08 +01:00

Originally created by @Koshub on GitHub (May 4, 2016).

Although it is only my opinion I think it is common problem I described in (old issue) when immediately loading data after update is not loaded updated values. That issue is not so correct because I had not seen the CoreStore internal design.

As a User I would like to expect that after the transaction has finished with commitAndWait method the UI context has already been updated and all its entities are updated from root context. Currently UI context is a child context by design, but it is not updated immediately after commitAndWait finished. There is handling NSManagedObjectContextDidSaveNotification and fetching with UI context is not waiting for NSManagedObjectContextDidSaveNotification processing finish. Also the NSManagedObjectContextObjectsDidChangeNotification is not handled but I think UI context as child of root context should be updated with this notification too.

I understand that current design is selected for performance reasons and currently UI is not waiting UI context mergings from root. Unfortunately I have to update every object I fetched with refreshAndMerge to show in UI. It is very easy to make a bug if I forget put refreshAndMerge somewhere in my code.

Is it possible to make commitAndWait method more predictable to allow user to choose if he prefers to merge transaction with UI context or not?
Thank you.

Originally created by @Koshub on GitHub (May 4, 2016). Although it is only my opinion I think it is common problem I described in [(old issue)](https://github.com/JohnEstropia/CoreStore/issues/62) when immediately loading data after update is not loaded updated values. That issue is not so correct because I had not seen the CoreStore internal design. As a User I would like to expect that after the transaction has finished with `commitAndWait` method the UI context has already been updated and all its entities are updated from root context. Currently UI context is a child context by design, but it is not updated immediately after `commitAndWait` finished. There is handling `NSManagedObjectContextDidSaveNotification` and fetching with UI context is not waiting for `NSManagedObjectContextDidSaveNotification` processing finish. Also the `NSManagedObjectContextObjectsDidChangeNotification` is not handled but I think UI context as child of root context should be updated with this notification too. I understand that current design is selected for performance reasons and currently UI is not waiting UI context mergings from root. Unfortunately I have to update every object I fetched with `refreshAndMerge` to show in UI. It is very easy to make a bug if I forget put `refreshAndMerge` somewhere in my code. Is it possible to make `commitAndWait` method more predictable to allow user to choose if he prefers to merge transaction with UI context or not? Thank you.

adam added the fixed corestore bug labels 2025-12-29 15:23:08 +01:00

adam closed this issue

2025-12-29 15:23:09 +01:00

adam commented

2025-12-29 15:23:09 +01:00

@omalovichko commented on GitHub (May 5, 2016):

It might be nice 👍

@omalovichko commented on GitHub (May 5, 2016): It might be nice 👍

adam commented

2025-12-29 15:23:11 +01:00

@JohnEstropia commented on GitHub (May 6, 2016):

Let me point out again that this is a only a problem when using synchronous transactions.

This is caused by the asynchronous context?.performBlock {...} handling in NSManagedObjectContextDidSaveNotification. I've tried the following workarounds below but unfortunately all of them have huge drawbacks.

1. Try using performBlockAndWait in the NSManagedObjectContextDidSaveNotification handler
This causes a deadlock when the UI does a fetch while an asynchronous transaction is saving. It's very easy to reproduce in one of our apps that downloads a user's history of chat messages continuously after reinstall.

2. Try calling refreshObject(_:mergeChanges:) on all the main context's registeredObjects inside commitAndWait()
This solves your problem, but the tradeoff looks bigger than the convenience it offers:

Since the main context does not know which objects are being updated (the updatedObjects property is empty at this point), we can only call refreshObject(_:mergeChanges:) on all the objects. If you have thousands of objects in memory, all of them will be refreshed and merged. We can try to refresh objects as faults which would be significantly cheaper, but that would cause transient property values to break and recompute.
NSFetchedResultsControllers (and ListMonitors and ObjectMonitors) get notified twice of the change, one from the NSManagedObjectContextDidSaveNotification merge and one from the explicit refresh.

3. Track the transaction's updatedObjects and call those objects' refreshObject(_:mergeChanges:) inside commitAndWait()
This is better than (2), but

NSFetchedResultsControllers still get notified twice for each change.
Since we selectively update objects, you may introduce conflicts in relationship values. (this is no different when calling object.refreshAndMerge() though)

I'm stuck here so far. Personally, I do not encourage synchronous transactions in the first place. I always recommend using ListMonitors and ObjectMonitors and pass those around instead of NSManagedObject instances themselves, then handle all changes through the observer delegates.

I do understand that this use case is sometimes unavoidable, so CoreStore exposes several refresh***() methods. So far I'm thinking of changing commitAndWait() to commitAndWait(refreshObjectsImmediately:) and with default value of refreshObjectsImmediately = false. If you have any other ideas, let me know.

@JohnEstropia commented on GitHub (May 6, 2016): Let me point out again that _this is a only a problem when using synchronous transactions._ This is caused by the asynchronous `context?.performBlock {...}` handling in `NSManagedObjectContextDidSaveNotification`. I've tried the following workarounds below but unfortunately all of them have huge drawbacks. **1. Try using `performBlockAndWait` in the `NSManagedObjectContextDidSaveNotification` handler** This causes a deadlock when the UI does a fetch while an asynchronous transaction is saving. It's very easy to reproduce in one of our apps that downloads a user's history of chat messages continuously after reinstall. **2. Try calling `refreshObject(_:mergeChanges:)` on all the main context's `registeredObjects` inside `commitAndWait()`** This solves your problem, but the tradeoff looks bigger than the convenience it offers: - Since the main context does not know which objects are being updated (the `updatedObjects` property is empty at this point), we can only call `refreshObject(_:mergeChanges:)` on all the objects. If you have thousands of objects in memory, all of them will be refreshed _and merged_. We can try to refresh objects as faults which would be significantly cheaper, but that would cause transient property values to break and recompute. - `NSFetchedResultsController`s (and `ListMonitor`s and `ObjectMonitor`s) get notified twice of the change, one from the `NSManagedObjectContextDidSaveNotification` merge and one from the explicit refresh. **3. Track the transaction's `updatedObjects` and call those objects' `refreshObject(_:mergeChanges:)` inside `commitAndWait()`** This is better than (2), but - `NSFetchedResultsController`s still get notified twice for each change. - Since we selectively update objects, you may introduce conflicts in relationship values. (this is no different when calling `object.refreshAndMerge()` though) I'm stuck here so far. Personally, I do not encourage synchronous transactions in the first place. I always recommend using `ListMonitor`s and `ObjectMonitor`s and pass those around instead of `NSManagedObject` instances themselves, then handle all changes through the observer delegates. I do understand that this use case is sometimes unavoidable, so CoreStore exposes several `refresh***()` methods. So far I'm thinking of changing `commitAndWait()` to `commitAndWait(refreshObjectsImmediately:)` and with default value of `refreshObjectsImmediately = false`. If you have any other ideas, let me know.

adam commented

2025-12-29 15:23:11 +01:00

@Koshub commented on GitHub (May 6, 2016):

Thank you for response. I'm currently thinking about better approach too.

@Koshub commented on GitHub (May 6, 2016): Thank you for response. I'm currently thinking about better approach too.

adam commented

2025-12-29 15:23:13 +01:00

@Koshub commented on GitHub (May 7, 2016):

Well I think that all existed stuff should be the same to make backward compatibility with avoiding NSFetchedResultsControllers and other listeners get notified twice. One needs to extend functionality probably based on the next ideas.

Every child context including UI context (because it is a child too) should have an ability to be configured to handle updates from his parent with NSManagedObjectContextDidSaveNotification notification with only performBlock call ( without performBlockAndWait). It gives an ability to make child be up to date without locks. Currently only UI context implements this thing but always and is not configured to turn off this functionality. Ability to configure child updates will gives more flexibility.

The next thing is to give parent context links with all its available child contexts. Then by improving saveSynchronously() (for example will be saveSynchronously(mergeChilds mergeChilds: Bool = false)) method of NSManagedObjectContext extension we could get all child contexts and after(or before) try self.save() start waiting for finishing all mergeChangesFromContextDidSaveNotification of child contexts which were configured to handle updates from their parent. After finish all blocks of child updates we continue saveSynchronously() to do other existence work (save parent if presented, etc).

Probably it could be hard to make some synchronisation of notifications queue ( of what save what notification sends) or/and hard to implement correctly, but I think it could make sense.

@Koshub commented on GitHub (May 7, 2016): Well I think that all existed stuff should be the same to make backward compatibility with avoiding `NSFetchedResultsControllers` and other listeners get notified twice. One needs to extend functionality probably based on the next ideas. Every child context including UI context (because it is a child too) should have an ability to be configured to handle updates from his parent with `NSManagedObjectContextDidSaveNotification` notification with only `performBlock` call ( without `performBlockAndWait`). It gives an ability to make child be up to date without locks. Currently only UI context implements this thing but always and is not configured to turn off this functionality. Ability to configure child updates will gives more flexibility. The next thing is to give parent context links with all its available child contexts. Then by improving `saveSynchronously()` (for example will be `saveSynchronously(mergeChilds mergeChilds: Bool = false)`) method of `NSManagedObjectContext` extension we could get all child contexts and after(or before) `try self.save()` start waiting for finishing all `mergeChangesFromContextDidSaveNotification` of child contexts which were configured to handle updates from their parent. After finish all blocks of child updates we continue `saveSynchronously()` to do other existence work (save parent if presented, etc). Probably it could be hard to make some synchronisation of notifications queue ( of what `save` what notification sends) or/and hard to implement correctly, but I think it could make sense.

adam commented

2025-12-29 15:23:13 +01:00

@JohnEstropia commented on GitHub (May 7, 2016):

Thanks for the ideas!

Every child context including UI context (because it is a child too) should have an ability to be configured to handle updates from his parent

This is unnecessary for transaction contexts because CoreStore guarantees that no transactions overlap (with the exception of unsafe transactions, which is why they're "unsafe"). This means that all transactions start with fresh and correct data state. And since the main context is read-only, you are guaranteed that no other transaction is meddling with the data. Check the README explanation of how the root context propagate data to the UI context and the transaction contexts: the UI context gets data from "merges" and the transaction contexts get data from the context-nesting mechanism.

The next thing is to give parent context links with all its available child contexts...

Your idea was on the right track. Maybe we can find a way to send an isSynchronous flag to NSManagedObjectContextDidSaveNotification so we can decide wether to use performBlock() or performBlockAndWait(). Something like

context.isSaveSynchronous = true // set with obj_setAssoc
context.save()
context.isSaveSynchronous = nil

I'll try it out later when I have access to my PC

@JohnEstropia commented on GitHub (May 7, 2016): Thanks for the ideas! > Every child context including UI context (because it is a child too) should have an ability to be configured to handle updates from his parent This is unnecessary for transaction contexts because CoreStore guarantees that no transactions overlap (with the exception of unsafe transactions, which is why they're "unsafe"). This means that all transactions start with fresh and correct data state. And since the main context is read-only, you are guaranteed that no other transaction is meddling with the data. Check the README explanation of how the root context propagate data to the UI context and the transaction contexts: the UI context gets data from "merges" and the transaction contexts get data from the context-nesting mechanism. > The next thing is to give parent context links with all its available child contexts... Your idea was on the right track. Maybe we can find a way to send an `isSynchronous` flag to `NSManagedObjectContextDidSaveNotification` so we can decide wether to use `performBlock()` or `performBlockAndWait()`. Something like ``` swift context.isSaveSynchronous = true // set with obj_setAssoc context.save() context.isSaveSynchronous = nil ``` I'll try it out later when I have access to my PC

adam commented

2025-12-29 15:23:13 +01:00

@JohnEstropia commented on GitHub (May 7, 2016):

It works! Try out the 1.6.5 tag and tell me how it goes for you :)

@JohnEstropia commented on GitHub (May 7, 2016): It works! Try out the `1.6.5` tag and tell me how it goes for you :)

adam commented

2025-12-29 15:23:14 +01:00

@JohnEstropia commented on GitHub (May 7, 2016):

Oh, no change needed on the caller side. Just update CoreStore and your current code should work as is

@JohnEstropia commented on GitHub (May 7, 2016): Oh, no change needed on the caller side. Just update CoreStore and your current code should work as is

adam commented

2025-12-29 15:23:14 +01:00

@Koshub commented on GitHub (May 7, 2016):

Great. Thank you for fix. Unfortunately I can't check it on real code, because I moved to plain CoreData management before I posted this issue. I will definitely check a bit later. According to changes in code I prefer blocking in sync function saveSynchronously to show that the function is real sync, but performBlockAndWait should work I think. Thank you for responses. Will let you know in any case.

@Koshub commented on GitHub (May 7, 2016): Great. Thank you for fix. Unfortunately I can't check it on real code, because I moved to plain CoreData management before I posted this issue. I will definitely check a bit later. According to changes in code I prefer blocking in sync function `saveSynchronously` to show that the function is real sync, but `performBlockAndWait` should work I think. Thank you for responses. Will let you know in any case.

adam commented

2025-12-29 15:23:14 +01:00

@JohnEstropia commented on GitHub (May 7, 2016):

@Koshub No problem. Thanks again!

You can confirm the fixed behavior with the CoreStoreDemo app.
Open the TransactionsDemoViewController.swift and change the refreshButtonTapped(_:) method to

    @IBAction dynamic func refreshButtonTapped(sender: AnyObject?) {

        let object = Static.placeController.object
        print(object)
        CoreStore.beginSynchronous { (transaction) -> Void in

            let place = transaction.edit(object)
            place?.setInitialValues()
            transaction.commitAndWait()
        }
        print(object)
    }

Prior to the fix, the two print() calls will display the same info. After the fix, the values set with setInitialValues() will be reflected on the second print() call.

@JohnEstropia commented on GitHub (May 7, 2016): @Koshub No problem. Thanks again! You can confirm the fixed behavior with the CoreStoreDemo app. Open the _TransactionsDemoViewController.swift_ and change the `refreshButtonTapped(_:)` method to ``` swift @IBAction dynamic func refreshButtonTapped(sender: AnyObject?) { let object = Static.placeController.object print(object) CoreStore.beginSynchronous { (transaction) -> Void in let place = transaction.edit(object) place?.setInitialValues() transaction.commitAndWait() } print(object) } ``` Prior to the fix, the two `print()` calls will display the same info. After the fix, the values set with `setInitialValues()` will be reflected on the second `print()` call.

adam commented

2025-12-29 15:23:14 +01:00

@Koshub commented on GitHub (May 7, 2016):

Perfect 👍

@Koshub commented on GitHub (May 7, 2016): Perfect 👍

adam referenced this issue

2025-12-29 15:32:12 +01:00

[PR #53] [MERGED] added missing closing bracket on sample code. #450

Sign in to join this conversation.

Branches Tags

develop

master

prototype/concurrency

minIOS13

prototype/containers

transactionSourceTracking

prototype/swiftui

minIOS11

swift5_3

prototype/propertyWrapperFields

prototype/propertyWrappers

prototype/propertyWrapper

Swift5

throwables

prototype/Swift_4_1

prototype/Swift_4_0

prototype/Swift_3_2

prototype/queryBuilders

temp/develop

corestore4_develop

prototype/mainContextToPSC

master_ios_7_to_9

swift_2_3

gh-pages

_corespotlight-test

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/CoreStore#53