Memory leak in Fetch

[malhal]

Hi Dave, sorry to let you know I noticed a leak in your Fetch DynamicProperty. Every time a View is re-init that contains @Fetch a new FetchObserver object is init but I think StateObject will hang onto on the first one thus any change to the filter will be ignored:

https://github.com/davedelong/extendedswift/blob/6c8e8c91bd7f9441aa5411f7b3e98eee6f41b0e1/Sources/ExtendedKit/CoreData/Fetch.swift#L44-L46

This can be fixed by changing: https://github.com/davedelong/extendedswift/blob/6c8e8c91bd7f9441aa5411f7b3e98eee6f41b0e1/Sources/ExtendedKit/CoreData/Fetch.swift#L15 to

@StateObject private var observer = FetchObserver<T>()

And then you can use update to give the observer the new filter and any other changed properties every time the dynamic property is re-init, .e.g

func update() {
    observer.update(from: self) // observer can then access the new filter value
}

Lastly now you can remove the filter public property since changes should always go through init.

[davedelong]

Hey @malhal, thanks for reporting this!

a couple of comments… 

• is this actually a leak? StateObject.init(wrappedValue:) will grab the parameter value in an @autoclosure and then only evaluate it if necessary.
• Currently, FetchObserver<T> requires having a Filter, which would preclude initializing it via FetchObserver<T>().

[malhal]

The @autoclosure requires the wrapped object init to be on same line, e.g. _observer = StateObject(wrappedValue: FetchObserver<T>(filter: filter, context: nil) ) That fixes the leak however you will still need to set a new filter on the object in the update func since the Fetch DynamicProperty might be init with a different one. You will need to save it in a let to bounce it over to update. Thus it's usually simpler to design the object to not have any required init params and only work via updates, since then you only have one code path.

[davedelong]

Yep, you're right about the same-line call; I've changed that.

FetchObserver requires the filter at initialization, because it can be used outside of a @Fetch property wrapper; it's a regular observable object, similar to NSFetchedResultsController. It is invalid to fetch something and not provide any information about which things you want to fetch.

[malhal]

One pattern to resolve that is throw an exception if the object is not configured correctly when results are being requested. I just noticed you have:

    func fetchIfNecessary() {

        guard let context else {
            print("FetchObserver<\(T.self)> is missing its ManagedObjectContext")
            return
        }

So you could throw an exception or return an error in the case of a missing context or a missing filter.