Weak self does nothing or what am i doing wrong?

I created a project to test [weak self]

There is:

So, in the code I use weak self, that makes ViewModel deinitialized, when user closes the ContentView. But if I open CPU graphs, there still are calculations, making to the end, even if this instance was deinitialized:

CPU and memory charts compiresment (with weak self and without)

If you compare this graphs with use of [weak self] and without it - there is absolutaly no difference.

Here is the whole code:

struct ContentView: View {
    @StateObject var vm = ViewModel()

    var body: some View {
        VStack {
            Text("App started")
            Text(vm.helloData.count.description)
        }
        .padding()
    }
}

class ViewModel: ObservableObject {
    @Published var helloData: [String] = []

    init() {
        print("ViewModel инициализирован")
        
        let count = UserDefaults.standard.integer(forKey: "count")
        UserDefaults.standard.set(count+1, forKey: "count")


        var data: [String] = []
        DispatchQueue.global().async { [weak self] in
            for _ in (0..<100_000_000) {
                data.append("Hello, world!")
            }
            DispatchQueue.main.sync {
                self?.helloData = data
            }
        }
    }

    deinit {
        print("ViewModel деинициализирован")

        let count = UserDefaults.standard.integer(forKey: "count")
        UserDefaults.standard.set(count-1, forKey: "count")
    }
}

struct ContentView: View {
    @StateObject var vm = ViewModel()

    var body: some View {
        VStack {
            Text("App started")
            Text(vm.helloData.count.description)
        }
        .padding()
    }
}

struct MenuView: View {
    @AppStorage ("count") var count: Int = 0

    init() {
        self.count = 0
    }

    var body: some View {
        NavigationStack{
            NavigationLink("go to ContentView", destination: ContentView())
        }
        .overlay {
            VStack {
                Text(count.description)
                    .font(.title)
                Spacer()
            }
        }
    }
}

Where am i wrong? As i understand, the reason of using weak self is to get rid of this useless calculations...

Upvotes: 0

Views: 144

Answers (3)

Rob
Rob

Reputation: 438152

I believe that JeremyP answered your immediate question (+1). I would advise accepting his answer.

That said, employing cancelation logic might be considered a more idiomatic solution than tests of whether view model was deallocated and whether its weak reference to itself was nil or not.

But this “add 100m strings” feels like it is a placeholder for something else. Unfortunately, the correct solution will vary based upon what you’re really doing. E.g., a bunch of asynchronous tasks begs for Swift concurrency approach. If the work really is slow and synchronous, we need to keep that out of the Swift concurrency cooperative thread pool (but bridge it back). GCD offers only the most basic cancelation support; Operation offers more elegant support; Swift concurrency offers first-class cancelation support, but really is only suitable if the work consists of async tasks.


Assuming for a second that the work really does consist of slow, synchronous calculations, I would first abstract that out of the view model and put it into its own object, an Operation in this case:

class StringsOperation: Operation {
    typealias ResultHandler = @Sendable (Result<[String], Error>) -> Void

    var resultHandler: ResultHandler?

    private let completionHandlerQueue: OperationQueue
    
    init(queue: OperationQueue = .main, resultHandler: ResultHandler? = nil) {
        self.resultHandler = resultHandler
        self.completionHandlerQueue = queue
    }
    
    override func main() {
        var data: [String] = []
        
        for _ in 0 ..< 100_000_000 {
            if isCancelled {
                completionHandlerQueue.addOperation { [weak self] in
                    self?.resultHandler?(.failure(CancellationError())) 
                    self?.resultHandler = nil
                }
                return 
            }
            data.append("Hello, world!")
        }

        completionHandlerQueue.addOperation { [weak self] in
            self?.resultHandler?(.success(data))
            self?.resultHandler = nil
        }
    }
}

The view model really should not be doing the complex calculations itself, but rather we have a nice, highly-cohesive object that encapsulates the calculation logic.

Note, I am checking for cancelation in the above Operation.

Then, the view model could add the operation to its own OperationQueue:

@MainActor
class ViewModel: ObservableObject {
    @Published var helloData: [String] = []
    
    private let queue = OperationQueue()
    
    init() {
        print("ViewModel инициализирован")
        startOperation()
    }
    
    deinit {
        print("ViewModel деинициализирован")
        queue.cancelAllOperations()
    }
}

private extension ViewModel {
    func startOperation() {
        let operation = StringsOperation { [weak self] result in
            MainActor.assumeIsolated { 
                switch result {
                case .success(let data):
                    self?.helloData = data
                case .failure(let error):
                    print(error)
                }
            }
        }
        
        queue.addOperation(operation)
    }
}

So, this starts the operation in init and cancels it in deinit.

As an aside, note that I isolated the ObservableObject view model to the main actor as advised by Apple in WWDC videos Swift concurrency: Update a sample app and Discover concurrency in SwiftUI.


Alternatively, instead of relying on the deinit of the view model, you could use a .task view modifier to start the work. Anything started in the .task view modifier will automatically be canceled when the view is dismissed.

So, first, the view model would bridge from these legacy patterns to Swift concurrency with withChecked[Throwing]Continuation and withTaskCancellationHandler:

@MainActor
class ViewModel: ObservableObject {
    @Published var helloData: [String] = []
    
    private let queue = OperationQueue()
    
    func start() async throws {
        let operation = StringsOperation()
        
        try await withTaskCancellationHandler {
            try Task.checkCancellation()
            
            helloData = try await withCheckedThrowingContinuation { continuation in
                operation.resultHandler = { result in
                    continuation.resume(with: result)
                }
                
                queue.addOperation(operation)
            }
        } onCancel: {
            operation.cancel()
        }
    }
}

And then the view would start the task in the .task view modifier:

struct ContentView: View {
    @StateObject var vm = ViewModel()
    
    var body: some View {
        VStack {
            Text("App started")
            Text(vm.helloData.count.description)
        }
        .padding()
        .task {
            do {
                try await vm.start()
            } catch {
                print(error)
            }
        }
    }
}

Note that I have refrained from moving this slow, synchronous operations into Swift concurrency tasks themselves. In WWDC 2021 video Swift concurrency: Behind the scenes Apple notes that we have a contract that we will never block a thread from Swift concurrency’s cooperative thread pool. So, in WWDC 2022 video Visualize and optimize Swift concurrency they explicitly advise keeping this sort of work out of the cooperative thread pool. We can easily bridge from these legacy patterns to Swift concurrency with withChecked[Throwing]Continuation and withTaskCancellationHandler, but I will not go there at this point.


As a final observation, I will note that if the work being initiated by the view controller was actually just time-consuming async functions, then the solution is even easier. None of that unintuitive bridging code would be needed.

Upvotes: 2

Cy-4AH
Cy-4AH

Reputation: 4585

You should use [weak self] in every nested closure. In yours DispatchQueue.main.sync { it's not weak already. It's just ViewModel?.

Compiler just making strong copies of all variables for closure by default.

If you have to many nested closures and afraid to forget about that somewhere better to just use weak let weakSelf = self instead.

By the way you don't need weak self for async: it will sometime finish and all dependencies will be released. You need weak self for closures that you store some where, for example in self's property.

Upvotes: 0

JeremyP
JeremyP

Reputation: 86671

As i understand, the reason of using weak self is to get rid of this useless calculations

No it is't. The point of using weak self is to break potential reference cycles. In your example, I don't think it's a problem anyway. The closure maintains a reference to self because of the assignment to self.helloData but there's no reference from self to the closure that I can see: the only thing that holds a reference to the closure will be the dispatch queue - or the task on it.

I assume by "useless calculations" you mean the appends to data inside the for loop, but data is a local variable, not a property: there's no reason why weak self would affect it. You can explicitly exit the loop if self does go away with a guard inside the loop e.g.

guard self != nil else { return } // or break

Incidentally, weak references impose a slight performance penalty because of the way they are implemented. IIRC when the instance is deallocated, all weak references have to be set to nil and I think it is done via an extra level of indirection, somehow.

Edit: the implementation is slightly more complex than I thought

https://www.mikeash.com/pyblog/friday-qa-2017-09-22-swift-4-weak-references.html

Upvotes: 4

Related Questions