Swift: How to deallocate properties when init throw?

Question

There is a memory leak with this example code.

The pointer1 and pointer2 allocate before Person inits successfully. If the init function throws an Error. The deinit function will never be executed. So the pointer1 and pointer2 will never be released.

import XCTest

class Person {

    // case1
    let pointer1: UnsafeMutablePointer<Int> = UnsafeMutablePointer<Int>.allocate(capacity: 1)

    // case2
    let pointer2: UnsafeMutablePointer<Int>

    let name: String

    init(name: String) throws {

        // case2
        self.pointer2 = UnsafeMutablePointer<Int>.allocate(capacity: 1)


        if name == "UnsupportName" {
            throw NSError()
        }
        self.name = name
    }

    deinit {
        pointer1.deallocate()
        pointer2.deallocate()
    }
}

class InterestTests: XCTestCase {

    func testExample() {
        while true {
            _ = try? Person(name: "UnsupportName")
        }
    }

}

Sometimes the logic is very complicated. In my real cases. There are a lot of allocate and throws with if and guard. Some it's hard to control it.

Is there any way to avoid this memory leak?

Here is a similar question: https://forums.swift.org/t/deinit-and-failable-initializers/1199

Answer 1

Another solution.

class Person {
    let name: String
    let pointer1: UnsafeMutablePointer<Int>
    let pointer2: UnsafeMutablePointer<Int>

    init(name: String) throws {
        var pointers: [UnsafeMutablePointer<Int>] = []
        do {
            let pointer1 = UnsafeMutablePointer<Int>.allocate(capacity: 1)
            pointers.append(pointer1)
            let pointer2 = UnsafeMutablePointer<Int>.allocate(capacity: 1)
            pointers.append(pointer2)
            if name == "Unsupported Name" {
                throw NSError()
            }
            self.pointer1 = pointer1
            self.pointer2 = pointer2
            self.name = name
        } catch {
            pointers.forEach { $0.deallocate() }
            throw error
        }
    }

    deinit {
        pointer1.deallocate()
        pointer2.deallocate()
    }
}

Answer 2

In your specific example, the solution is straightforward. Don't allocate any memory until you have resolved all possible failures:

class Person {

    let aPointer: UnsafeMutablePointer<Int> // Do not allocate here.
    let name: String

    init(name: String) throws {
        // Validate everything here
        guard name != "UnsupportName" else {
            throw NSError()
        }

        // After this point, no more throwing:

        self.name = name
        // Move the allocation here
        self.aPointer = UnsafeMutablePointer.allocate(capacity: 1)
    }

    deinit {
        aPointer.deallocate()
    }
}

But the more general solution is to use do/catch like anywhere else you need to manage errors:

class Person {

    let aPointer = UnsafeMutablePointer<Int>.allocate(capacity: 1)
    let name: String

    init(name: String) throws {
        do {
            if name == "UnsupportName" {
                throw NSError()
            }

            self.name = name
        } catch let e {
            self.aPointer.deallocate()
            throw e
        }

    }

    deinit {
        aPointer.deallocate()
    }
}

I would be tempted to move the .allocate inside the init, just to make it a bit more visible what's going on. The key point is that you should either allocate all your memory first, before anything can throw (so you know you can deallocate it all), or all after the last throw (so you know you don't have anything to deallocate).

---

Looking at the solution you've added, it's ok, but suggests dangerous logic surrounding it. It would be much better to unwind this to place the allocation into their own objects (which would almost certainly also get rid of the UnsafeMutablePointers; needing a lot of those in a single class is very suspicious).

That said, there are cleaner ways IMO to build the error handling along this path.

extension UnsafeMutablePointer {
    static func allocate(capacity: Int, withCleanup cleanup: inout [() -> Void]) -> UnsafeMutablePointer<Pointee> {
        let result = allocate(capacity: capacity)
        result.addTo(cleanup: &cleanup)
        return result
    }

    func addTo(cleanup: inout [() -> Void]) {
        cleanup.append { self.deallocate() }
    }
}

This lets UnsafeMutablePointers append cleanup information into an array, rather than creating a lot of defer blocks, which raises the risk of missing one during cleanup.

With that, your init looks like:

init(name: String) throws {
    var errorCleanup: [() -> Void] = []
    defer { for cleanup in errorCleanup { cleanup() } }

    // deallocate helper for case1
    pointer1.addTo(cleanup: &errorCleanup)

    // case2
    self.pointer2 = UnsafeMutablePointer<Int>.allocate(capacity: 1, withCleanup: &errorCleanup)

    // case ...


    if name == "UnsupportName" {
        throw NSError()
    }
    self.name = name

    // In the end. set deallocate helpers to nil
    errorCleanup.removeAll()
}

Of course that setups the danger of calling allocate(capacity:) rather than allocate(capacity:withCleanup:). So you could fix that by wrapping it into another type; a reference type that automatically deallocates itself.

class SharedPointer<Pointee> {
    let ptr: UnsafeMutablePointer<Pointee>
    static func allocate(capacity: Int) -> SharedPointer {
        return .init(pointer: UnsafeMutablePointer.allocate(capacity: capacity))
    }
    init(pointer: UnsafeMutablePointer<Pointee>) {
        self.ptr = pointer
    }
    deinit {
        ptr.deallocate()
    }
}

With that, this becomes (no deinit required):

class Person {

    // case1
    let pointer1 = SharedPointer<Int>.allocate(capacity: 1)

    // case2
    let pointer2: SharedPointer<Int>

    let name: String

    init(name: String) throws {

        // case2
        self.pointer2 = SharedPointer<Int>.allocate(capacity: 1)

        if name == "UnsupportName" {
            throw NSError()
        }
        self.name = name
    }
}

You'd probably want to write various helpers for dealing with .ptr.

Of course this might lead you to build specific versions of SharedPointer to deal with each kind of thing (like "father" rather than "int"). If you continue down that road, you'll find the UnsafeMutablePointers evaporate, and the problem goes away. But you don't have to go that far, and SharedPointer will do the work for you.

Answer 3

I found a solution to my problem.

import XCTest

class Person {

    // case1
    let pointer1: UnsafeMutablePointer<Int> = UnsafeMutablePointer<Int>.allocate(capacity: 1)

    // case2
    let pointer2: UnsafeMutablePointer<Int>

    let name: String

    init(name: String) throws {

        // deallocate helper for case1
        var deallocateHelper1: UnsafeMutablePointer<Int>? = self.pointer1
        defer {
            deallocateHelper1?.deallocate()
        }

        // case2
        self.pointer2 = UnsafeMutablePointer<Int>.allocate(capacity: 1)
        var deallocateHelper2: UnsafeMutablePointer<Int>? = self.pointer2
        defer {
            deallocateHelper2?.deallocate()
        }

        // case ... 


        if name == "UnsupportName" {
            throw NSError()
        }
        self.name = name

        // In the end. set deallocate helpers to nil
        deallocateHelper1 = nil
        deallocateHelper2 = nil
    }

    deinit {
        pointer1.deallocate()
        pointer2.deallocate()
    }
}

class InterestTests: XCTestCase {

    func testExample() {
        while true {
            _ = try? Person(name: "UnsupportName")
        }
    }

}