Add .noOpRetain() to all elided types

Swift is very eager to deallocate objects, even before their last use. For example, take this code block in Listen.swift:

/// Notifies the listener that a block was added at the given height.
public override func blockConnected(block: [UInt8], height: UInt32) {
    // native call variable prep

    let blockPrimitiveWrapper = u8slice(value: block)

    // native method call
    let nativeCallResult = self.cType!.block_connected(self.cType!.this_arg, blockPrimitiveWrapper.cType!, height)

    // cleanup

    // for elided types, we need this
    blockPrimitiveWrapper.noOpRetain()

    // return value (do some wrapping)
    let returnValue = nativeCallResult

    return returnValue
}

Here, blockPrimitiveWrapper gets deinit before the let nativeCallResult = self.cType!.block_connected call. Presumably Swift simply store the value of blockPrimitiveWrapper.cType behind the scenes, and then just passes that. Because a deinit cleans up the contents, it invalidates the cType, and we have an access of deallocated memory.

This is only a concern when we instantiate the wrapper inside the method, i. e. it's a concern for all elided types. However, if we then take a look at this method:

public class func swiftFindRoute(ourNodePubkey: [UInt8], routeParams: RouteParameters, networkGraph: NetworkGraph, firstHops: [ChannelDetails]?, logger: Logger, scorer: Score, randomSeedBytes: [UInt8]) -> Result_RouteLightningErrorZ {
    // native call variable prep

    let ourNodePubkeyPrimitiveWrapper = PublicKey(value: ourNodePubkey)

    var firstHopsVectorPointer: UnsafeMutablePointer<LDKCVec_ChannelDetailsZ>? = nil
    if let firstHops = firstHops {

        let firstHopsVector = Vec_ChannelDetailsZ(array: firstHops).dangle()

        firstHopsVectorPointer = UnsafeMutablePointer<LDKCVec_ChannelDetailsZ>.allocate(capacity: 1)
        firstHopsVectorPointer!.initialize(to: firstHopsVector.cType!)
    }

    let tupledRandomSeedBytes = Bindings.arrayToUInt8Tuple32(array: randomSeedBytes)

    // native method call
    let nativeCallResult =
        withUnsafePointer(to: routeParams.dynamicallyDangledClone().cType!) { (routeParamsPointer: UnsafePointer<LDKRouteParameters>) in

            withUnsafePointer(to: networkGraph.dangle().cType!) { (networkGraphPointer: UnsafePointer<LDKNetworkGraph>) in

                withUnsafePointer(to: scorer.activate().cType!) { (scorerPointer: UnsafePointer<LDKScore>) in

                    withUnsafePointer(to: tupledRandomSeedBytes) { (tupledRandomSeedBytesPointer: UnsafePointer<UInt8Tuple32>) in
                        find_route(ourNodePubkeyPrimitiveWrapper.cType!, routeParamsPointer, networkGraphPointer, firstHopsVectorPointer, logger.activate().cType!, scorerPointer, tupledRandomSeedBytesPointer)
                    }

                }

            }

        }

    // cleanup

    // for elided types, we need this
    ourNodePubkeyPrimitiveWrapper.noOpRetain()

    firstHopsVector.noOpRetain()

    // return value (do some wrapping)
    let returnValue = Result_RouteLightningErrorZ(cType: nativeCallResult)

    return returnValue
}

We find that we're trying to do a noOpRetain on firstHopsVector, which of course we should be doing, but we can't, because its argument was an optional, and it therefore only exists inside the if let block. Additionally, it doesn't really make a difference because the object was dangled prior to being passed, so whether or not Swift calls deinit, nothing actually happens to the underlying C value, and we're safe.

However, for the purposes of future-proofing, or if we should choose not to dangle the object, we should instead be able to do a noOpRetain on that object, as well. The same applies to tuples, of course.

As of today, it just so happens that we don't have any primitive wrappers being passed as optionals, so we can punt on that for the time being, but we should fix that in the medium term.

arik-so / ldk-parser

Add .noOpRetain() to all elided types #21