Memory leak and a crash when swizzling NSURLRequest initialiser

When swizzling NSURLRequest initialiser and returning a mutable copy, the original instance does not get deallocated and eventually gets leaked and a crash follows after that.

Here's the swizzling setup:

static func swizzleInit() {
    let initSel = NSSelectorFromString("initWithURL:cachePolicy:timeoutInterval:")

    guard let initMethod = class_getInstanceMethod(NSClassFromString("NSURLRequest"), initSel) else {
        return
    }

    let origInitImp = method_getImplementation(initMethod)

    let block: @convention(block) (AnyObject, Any, NSURLRequest.CachePolicy, TimeInterval) -> NSURLRequest = { _self, url, policy, interval in
        typealias OrigInit = @convention(c) (AnyObject, Selector, Any, NSURLRequest.CachePolicy, TimeInterval) -> NSURLRequest
        let origFunc = unsafeBitCast(origInitImp, to: OrigInit.self)
        let request = origFunc(_self, initSel, url, policy, interval)

        return request.tagged()
    }

    let newImplementation = imp_implementationWithBlock(block as Any)
    method_setImplementation(initMethod, newImplementation)
}

// create a mutable copy if needed and add a header
private func tagged() -> NSURLRequest {
    guard let mutableRequest = self as? NSMutableURLRequest ?? self.mutableCopy() as? NSMutableURLRequest else {
        return self
    }
    mutableRequest.setValue("test", forHTTPHeaderField: "test")
    return mutableRequest
}

Then, we have a few test cases:

// memory leak and crash
func testSwizzleNSURLRequestInit() {
    let request = NSURLRequest(url: URL(string: "https://example.com")!)
    XCTAssertEqual(request.value(forHTTPHeaderField: "test"), "test")
}

// no crash, as the request is mutable, so no copy is created
func testSwizzleNSURLRequestInit2() {
    let request = URLRequest(url: URL(string: "https://example.com")!)
    XCTAssertEqual(request.value(forHTTPHeaderField: "test"), "test")
}

// no crash, as the request is mutable, so no copy is created
func testSwizzleNSURLRequestInit3() {
    let request = NSMutableURLRequest(url: URL(string: "https://example.com")!)
    XCTAssertEqual(request.value(forHTTPHeaderField: "test"), "test")
}

// no crash, as the new instance does not get deallocated 
// when the test method completes (?)
var request: NSURLRequest?
func testSwizzleNSURLRequestInit4() {
    request = NSURLRequest(url: URL(string: "https://example.com")!)
    XCTAssertEqual(request?.value(forHTTPHeaderField: "test"), "test")
}

It appears a memory leak occurs only when any other instance except for the original one is being returned from the initialiser.

Is there a workaround to prevent the leak, while allowing for modifications of all requests?

Answered by DTS Engineer in 808126022

What Kevin said plus…

I want to be 100% clear that swizzling methods in system classes is not supported. You may get this working, but your code will inevitably depend on implementation details that might cause it to break in the future. I strongly recommend that you reconsider your overall path.

For context, Apple has rewritten a huge chunk of Foundation in Swift over the past few years. If you’re curious about the details, watch Swift & Interoperability

https://youtu.be/%77%6e6C_XEv1Mo

In the face of such a large and ongoing refactoring, most apps will continue running just fine. That’s because they work with our APIs. Where things go wrong is when you start relying on implementation details.

In this specific case, things are every more complicated because of the relationship between Swift’s URLRequest, Foundation’s NSURLRequest, and CFNetwork’s private implementation classes. I talk about this in some detail in this post.

If you care to share details about why you’re doing this, I’d be happy to discuss your alternatives.

Share and Enjoy

Quinn “The Eskimo!” @ Developer Technical Support @ Apple
let myEmail = "eskimo" + "1" + "@" + "apple.com"

Hi,

When swizzling NSURLRequest initialiser and returning a mutable copy,

This is a really bad idea. NSURLRequest is a very widely used class and the consequence of this kind of wide scale alternation are very difficult to predict. As the most obvious example, I think what you're doing here just broke any code that subclassed "NSURLRequest".

the original instance does not get deallocated and eventually gets leaked and a crash follows after that.

Yes, I suspect you're not deallocating the originally allocated object, but it might also be one of the object internal to NSURLRequest. I'm not entirely sure which it would be, as there is some complicated stuff going on here since:

  • You're calling into an object ("self") that hasn't actually been initialized (since you haven't called "self.init").

  • You're also re-entering init, since "mutableCopy" calls back into init.

  • NSURLRequest is somewhat of a class cluster, which means it's internal initialization isn't as straightforward as you might think.

__
Kevin Elliott
DTS Engineer, CoreOS/Hardware

What Kevin said plus…

I want to be 100% clear that swizzling methods in system classes is not supported. You may get this working, but your code will inevitably depend on implementation details that might cause it to break in the future. I strongly recommend that you reconsider your overall path.

For context, Apple has rewritten a huge chunk of Foundation in Swift over the past few years. If you’re curious about the details, watch Swift & Interoperability

https://youtu.be/%77%6e6C_XEv1Mo

In the face of such a large and ongoing refactoring, most apps will continue running just fine. That’s because they work with our APIs. Where things go wrong is when you start relying on implementation details.

In this specific case, things are every more complicated because of the relationship between Swift’s URLRequest, Foundation’s NSURLRequest, and CFNetwork’s private implementation classes. I talk about this in some detail in this post.

If you care to share details about why you’re doing this, I’d be happy to discuss your alternatives.

Share and Enjoy

Quinn “The Eskimo!” @ Developer Technical Support @ Apple
let myEmail = "eskimo" + "1" + "@" + "apple.com"

Please reply as a reply; if you reply in the comments, I may not see it. For this and other titbits, see Quinn’s Top Ten DevForums Tips.

I’d be happy to provide more details and discuss the alternatives outside of the forum

Fair enough. I’ve pulled that into my queue and I’ll reach out to you that way soon (hopefully later today).

Share and Enjoy

Quinn “The Eskimo!” @ Developer Technical Support @ Apple
let myEmail = "eskimo" + "1" + "@" + "apple.com"

Memory leak and a crash when swizzling NSURLRequest initialiser
 
 
Q