fetchExisting() crashes the app when object is not found #90

New Issue

Closed

opened 2025-12-29 15:24:14 +01:00 by adam · 11 comments

adam commented 2025-12-29 15:24:14 +01:00

Owner

Copy Link

Originally created by @mkadan on GitHub (Sep 29, 2016).

please check DataStack+Querying.swift and BaseDataTransaction+Querying.swift

Four fetchExisting() methods there have the "as!" cast, e.g.:

return (try self.context.existingObjectWithID(object.objectID) as! T)

This cast crashes the app if nil is casted (that may happen if the object is not available in the context), and this crash if not catched, as this is a fatal crash.

existingObjectWithID
If the object cannot be fetched, or does not exist, or cannot be faulted, it returns nil.

Why not use "as?" instead?

---

A bit another, though related issue is if the object was never saved, then the app crashes with:

2016-09-29 12:19:17.898 LayfootakUserApp[55561:7950554] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'nil is not a valid object ID'

In my opinion it should not crash and just return nil.
Could easily be replacing it with this in singe object fetchExisting() methods:

return (object.objectID.persistentStore != nil) ? (try self.context.existingObjectWithID(object.objectID) as? T) : nil

and with this in multi object fetchExisting() methods:

return objects.flatMap { ($0.objectID.persistentStore != nil) ? (try? self.mainContext.existingObjectWithID($0.objectID)) as? T : nil }

---

Here I attach fixed sources that resolve these two issues:
fixed-sources.zip

Originally created by @mkadan on GitHub (Sep 29, 2016). please check DataStack+Querying.swift and BaseDataTransaction+Querying.swift Four fetchExisting() methods there have the "as!" cast, e.g.: > return (try self.context.existingObjectWithID(object.objectID) as! T) This cast crashes the app if nil is casted (that may happen if the object is not available in the context), and this crash if not catched, as this is a fatal crash. > existingObjectWithID > If the object cannot be fetched, or does not exist, or cannot be faulted, it returns nil. Why not use "as?" instead? --- A bit another, though related issue is if the object was never saved, then the app crashes with: > 2016-09-29 12:19:17.898 LayfootakUserApp[55561:7950554] **\* Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'nil is not a valid object ID' In my opinion it should not crash and just return nil. Could easily be replacing it with this in singe object fetchExisting() methods: > return (object.objectID.persistentStore != nil) ? (try self.context.existingObjectWithID(object.objectID) as? T) : nil and with this in multi object fetchExisting() methods: > return objects.flatMap { ($0.objectID.persistentStore != nil) ? (try? self.mainContext.existingObjectWithID($0.objectID)) as? T : nil } --- Here I attach fixed sources that resolve these two issues: [fixed-sources.zip](https://github.com/JohnEstropia/CoreStore/files/500483/fixed-sources.zip)

adam added the user bug label 2025-12-29 15:24:14 +01:00

adam closed this issue 2025-12-29 15:24:15 +01:00

adam commented 2025-12-29 15:24:16 +01:00

Author

Owner

Copy Link

@JohnEstropia commented on GitHub (Sep 29, 2016):

@mkadan The documentation you are referring to is for Objective-C

If the object cannot be fetched, or does not exist, or cannot be faulted, it returns nil.

In Objective-C, errors are returned from methods by returning nil and populating the error parameter:

- (__kindof NSManagedObject *)existingObjectWithID:(NSManagedObjectID *)objectID 
                                             error:(NSError * _Nullable *)error;

In Swift, this method does not return an optional, and so all nil returns will be bridged by swift as a throw:

func existingObject(with objectID: NSManagedObjectID) throws -> NSManagedObject

existingObject() 's documentation:
https://developer.apple.com/reference/coredata/nsmanagedobjectcontext/1506686-existingobject

The as! T casting done by CoreStore just casts the non-optional NSManagedObject to T.

A bit another, though related issue is if the object was never saved, then the app crashes with:
2016-09-29 12:19:17.898 LayfootakUserApp[55561:7950554] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'nil is not a valid object ID'

None of CoreStore's fetchExisting() methods accepts an optional. So if you are passing nil there then Swift has nothing else to do but crash. I am guessing that a nil value from your Objective-C code is getting through Swift bridged code, so you should start to debug where that happens.

@JohnEstropia commented on GitHub (Sep 29, 2016): @mkadan The documentation you are referring to is for Objective-C ``` If the object cannot be fetched, or does not exist, or cannot be faulted, it returns nil. ``` In Objective-C, errors are returned from methods by returning `nil` and populating the `error` parameter: ``` objc - (__kindof NSManagedObject *)existingObjectWithID:(NSManagedObjectID *)objectID error:(NSError * _Nullable *)error; ``` In Swift, this method does not return an optional, and so all `nil` returns will be bridged by swift as a `throw`: ``` swift func existingObject(with objectID: NSManagedObjectID) throws -> NSManagedObject ``` `existingObject()` 's documentation: https://developer.apple.com/reference/coredata/nsmanagedobjectcontext/1506686-existingobject The `as! T` casting done by CoreStore just casts the non-optional `NSManagedObject` to `T`. > A bit another, though related issue is if the object was never saved, then the app crashes with: > 2016-09-29 12:19:17.898 LayfootakUserApp[55561:7950554] **\* Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'nil is not a valid object ID' None of CoreStore's `fetchExisting()` methods accepts an optional. So if you are passing `nil` there then Swift has nothing else to do but crash. I am guessing that a `nil` value from your Objective-C code is getting through Swift bridged code, so you should start to debug where that happens.

adam commented 2025-12-29 15:24:17 +01:00

Author

Owner

Copy Link

@mkadan commented on GitHub (Sep 29, 2016):

Ok, I see your point for the issue N1, and if it works like this, that's ok for me.

But please check this sample code for issue N2, it crashes, though in my opinion it should not:

    CoreStore.beginAsynchronous {
        (transaction) in
        let user = User()
        let user$ = transaction.fetchExisting(user)
    }

It may happen not only if the instance was just created but also when it was deleted, like it happens in our app.

@mkadan commented on GitHub (Sep 29, 2016): Ok, I see your point for the issue N1, and if it works like this, that's ok for me. But please check this sample code for issue N2, it crashes, though in my opinion it should not: > ``` > CoreStore.beginAsynchronous { > (transaction) in > let user = User() > let user$ = transaction.fetchExisting(user) > } > ``` It may happen not only if the instance was just created but also when it was deleted, like it happens in our app.

adam commented 2025-12-29 15:24:17 +01:00

Author

Owner

Copy Link

@JohnEstropia commented on GitHub (Sep 29, 2016):

You can't create NSManagedObject's directly through their default initializer. That object won't have a parent NSManagedObjectContext. Use the transaction's create method for this:

CoreStore.beginAsynchronous {
        (transaction) in
    let user = transaction.create(Into<User>())
}

@JohnEstropia commented on GitHub (Sep 29, 2016): You can't create `NSManagedObject`'s directly through their default initializer. That object won't have a parent `NSManagedObjectContext`. Use the `transaction`'s create method for this: ``` swift CoreStore.beginAsynchronous { (transaction) in let user = transaction.create(Into<User>()) } ```

adam commented 2025-12-29 15:24:17 +01:00

Author

Owner

Copy Link

@mkadan commented on GitHub (Sep 29, 2016):

Ok thanks for your explanation. I tried to create a sample based on our code that fails, but seems it's not that easy to do, as it involves cascaded deletes. I'll provide it later when I extract the problem, of course if that's really a problem in CoreStore.

@mkadan commented on GitHub (Sep 29, 2016): Ok thanks for your explanation. I tried to create a sample based on our code that fails, but seems it's not that easy to do, as it involves cascaded deletes. I'll provide it later when I extract the problem, of course if that's really a problem in CoreStore.

adam commented 2025-12-29 15:24:17 +01:00

Author

Owner

Copy Link

@mkadan commented on GitHub (Sep 29, 2016):

I was able to reproduce the problem on sample project:

Company has ToMany-DeleteCascade relation to User, User has ToOne-NullifyCascade relation to Company.

    CoreStore.beginSynchronous {
        (transaction) in
        let user$ = transaction.create(Into(User))
        let company$ = transaction.create(Into(Company))

        user$.company = company$

        transaction.commitAndWait()
    }

    let user = CoreStore.fetchOne(From(User))!

    CoreStore.beginSynchronous {
        (transaction) in
        let user$ = transaction.fetchExisting(user)
        transaction.delete(user$)

        transaction.commitAndWait()
    }

    let company = user.company
    // it's objectID is 00000000000000000000 and has no persistenceStore set

    CoreStore.beginSynchronous {
        (transaction) in
        // crashes
        let company$ = transaction.fetchExisting(company)

        transaction.commitAndWait()
    }

using

return (object.objectID.persistentStore != nil) ? (try self.context.existingObjectWithID(object.objectID) as! T) : nil

in fetchExisting() implementation fixes this crash

@mkadan commented on GitHub (Sep 29, 2016): I was able to reproduce the problem on sample project: Company has ToMany-DeleteCascade relation to User, User has ToOne-NullifyCascade relation to Company. > ``` > CoreStore.beginSynchronous { > (transaction) in > let user$ = transaction.create(Into(User)) > let company$ = transaction.create(Into(Company)) > > user$.company = company$ > > transaction.commitAndWait() > } > > let user = CoreStore.fetchOne(From(User))! > > CoreStore.beginSynchronous { > (transaction) in > let user$ = transaction.fetchExisting(user) > transaction.delete(user$) > > transaction.commitAndWait() > } > > let company = user.company > // it's objectID is 00000000000000000000 and has no persistenceStore set > > CoreStore.beginSynchronous { > (transaction) in > // crashes > let company$ = transaction.fetchExisting(company) > > transaction.commitAndWait() > } > ``` using > return (object.objectID.persistentStore != nil) ? (try self.context.existingObjectWithID(object.objectID) as! T) : nil in fetchExisting() implementation fixes this crash

adam commented 2025-12-29 15:24:18 +01:00

Author

Owner

Copy Link

@JohnEstropia commented on GitHub (Sep 29, 2016):

Thats expected behavior. You accessed an object that is supposedly deleted. My guess is that user.community is supposed to return nil, but the Swift compiler cannot infer that for you during compile time, so you end up with a broken 0-object. In any case, CoreStore can't protect you from accessing deleted objects directly like this, as that is Core Data's internal behavior

@JohnEstropia commented on GitHub (Sep 29, 2016): Thats expected behavior. You accessed an object that is supposedly deleted. My guess is that user.community is supposed to return nil, but the Swift compiler cannot infer that for you during compile time, so you end up with a broken 0-object. In any case, CoreStore can't protect you from accessing deleted objects directly like this, as that is Core Data's internal behavior

adam commented 2025-12-29 15:24:18 +01:00

Author

Owner

Copy Link

@mkadan commented on GitHub (Sep 29, 2016):

But is it possible to add this check to CoreStore just as a workaround for CoreData bug?

@mkadan commented on GitHub (Sep 29, 2016): But is it possible to add this check to CoreStore just as a workaround for CoreData bug?

adam commented 2025-12-29 15:24:19 +01:00

Author

Owner

Copy Link

@JohnEstropia commented on GitHub (Sep 29, 2016):

That's not exactly a Core Data bug. It is the responsibility of the app to design an architecture so this doesn't happen. If anything, CoreStore should be guarding against this with an assert(!object.isDeleted), because tolerating this will most likely just cause another problem somewhere else. On iOS 8 and below, your code will probably already crash on the line

 let company = user.company

(iOS 9's new shouldDeleteInaccessibleFaults just keeps the object up for a while)

In any case, I suggest that this be handled on the app side.

@JohnEstropia commented on GitHub (Sep 29, 2016): That's not exactly a Core Data bug. It is the responsibility of the app to design an architecture so this doesn't happen. If anything, CoreStore should be guarding against this with an `assert(!object.isDeleted)`, because tolerating this will most likely just cause another problem somewhere else. On iOS 8 and below, your code will probably already crash on the line ``` let company = user.company ``` (iOS 9's new `shouldDeleteInaccessibleFaults` just keeps the object up for a while) In any case, I suggest that this be handled on the app side.

adam commented 2025-12-29 15:24:19 +01:00

Author

Owner

Copy Link

@mkadan commented on GitHub (Sep 29, 2016):

Thanks for your answer. I think we will try to review our code to exclude such cases from it, and if it is not possible for some reason, will write an extension method to CoreStore transaction to perform this check outside of the core library functionality and test it carefully for our app, as you have a point that it may break something else inside the CoreStore.
From your side, is it possible you include this assert with some reasonable message, so that users may know what actually caused the crash?

@mkadan commented on GitHub (Sep 29, 2016): Thanks for your answer. I think we will try to review our code to exclude such cases from it, and if it is not possible for some reason, will write an extension method to CoreStore transaction to perform this check outside of the core library functionality and test it carefully for our app, as you have a point that it may break something else inside the CoreStore. From your side, is it possible you include this assert with some reasonable message, so that users may know what actually caused the crash?

adam commented 2025-12-29 15:24:19 +01:00

Author

Owner

Copy Link

@JohnEstropia commented on GitHub (Sep 29, 2016):

We could do that, but iOS 9 should already be giving you a warning log when this happens. Also, on iOS 8 your app will be crashing with the original exception anyway.

@JohnEstropia commented on GitHub (Sep 29, 2016): We could do that, but iOS 9 should already be [giving you a warning log](http://stackoverflow.com/a/32503486/809614) when this happens. Also, on iOS 8 your app will be crashing with the original exception anyway.

adam commented 2025-12-29 15:24:20 +01:00

Author

Owner

Copy Link

@JohnEstropia commented on GitHub (Sep 29, 2016):

I'll experiment for a while with that assert(). If it changes the current behavior of fetchExisting(), I might not put it in (example, if there are previous cases where isDeleted is marked but goes through fetchExisting() returning nil, adding an assert() will be a breaking change for those use cases)

@JohnEstropia commented on GitHub (Sep 29, 2016): I'll experiment for a while with that `assert()`. If it changes the current behavior of fetchExisting(), I might not put it in (example, if there are previous cases where `isDeleted` is marked but goes through `fetchExisting()` returning nil, adding an `assert()` will be a breaking change for those use cases)

Sign in to join this conversation.

No Branch/Tag Specified

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

9.3.0

9.2.0

9.1.0

9.0.0

8.1.0

8.0.1

8.0.0

7.3.1

7.3.0

7.2.0

7.1.0

7.0.4

7.0.3

7.0.2

7.0.1

7.0.0

6.3.2

6.3.1

6.3.0

6.2.1

6.2.0

6.1.0

6.0.0

5.3.1

5.3.0

5.2.0

5.1.1

5.1.0

5.0.0

4.2.4

4.2.3

4.2.2

4.2.1

4.2.0

4.1.4

4.1.3

4.1.1

4.1.0

4.0.5

4.0.4

4.0.3

4.0.2

4.0.1

4.0.0

4.0.0-beta3

4.0.0-beta2

4.0.0-beta

3.1.1

3.1.0

3.0.3

3.0.2

3.0.1

3.0.0

2.1.3

2.1.2

2.1.1

2.1.0

2.0.6

2.0.5

2.0.4

2.0.3

2.0.2

2.0.1

2.0.0

1.6.10

1.6.9

1.6.8

1.6.7

1.6.6

1.6.5

1.6.4

1.6.3

1.6.2

1.6.1

1.6.0

1.5.4

1.5.3

1.5.2

1.5.1

1.5.0

1.4.4

1.4.3

1.4.2

1.4.1

1.4.0

1.3.6

1.3.5

1.3.4

1.3.3

1.3.2

1.3.1

1.3.0

1.2.1

1.2.0

1.0.2

1.0.1

1.0.0

0.2.1

0.2.0

0.1.2

0.1.1

0.1.0

Labels

Clear labels

abandoned

corestore bug

discussion

duplicate

enhancement

fixed

fixed

help wanted

in progress

investigating

ios/compiler bug

irreproducible

looking for workarounds

not a bug

pending

pending docs update

pull-request

Mirrored from GitHub Pull Request

question

regression

user bug

wontfix

No Label user bug

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

adam

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/CoreStore#90