New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible memory leak adding many background download requests in a loop #1938

Closed
unusuallyreticent opened this Issue Jan 29, 2017 · 23 comments

Comments

Projects
None yet
4 participants
@unusuallyreticent

unusuallyreticent commented Jan 29, 2017

Excuse the last post - accidentally hit something...

I have a strange issue with Instruments leaks. I'm running a loop to add many download requests to a background session. The SessionManager is contained in a class singleton pretty much as follows:

class BackgroundTransferCoordinator {
static let sharedInstance = BackgroundTransferCoordinator()
lazy var session: SessionManager = {
// session with background configuration
}()
// .. too much to show
}

When building background requests in a loop, the following code results in a stunning number of leaks after adding just four requests:

for _ in 0..<4 {
let request = self.session.download(url)
request.resume()
}

To confuse matters, adding less than 4 items is more than often OK. Replace the for loop content with this:

let request = self.session.session.downloadTask(with: url)
request.resume()

...and you can add as any requests as you like, with no leaks showing up. I'm typically adding 50 or more requests on a first run and then adding a couple per day thereafter - documents.

The BackgroundTransferCoordinator class sets handlers for:
session.delegate.taskDidComplete
session.delegate.downloadDidFinishDownloadingToURL

I actually run the code inside the getAllTasks completion handler to query existing background tasks and then add missing tasks. However, other code builds requests on the main queue and that also fails after adding more than three in a loop. There is no crash occurring, and all requests complete properly - got the files to prove it.

As far as I can see, the difference appears to be the second example, by using the URLSession directly, bypasses this:

private func download( _ downloadable: DownloadRequest.Downloadable,
to destination: DownloadRequest.DownloadFileDestination?)
-> DownloadRequest

... and requests do not end up being added to SessionDelegate.requests

I'm not convinced this isn't an issue with Instruments or the way I'm using it. I will try to make a test project in the next couple of days if possible.

@jshier

This comment has been minimized.

Show comment
Hide comment
@jshier

jshier Jan 30, 2017

Contributor

Can you post the leaks output or the lines it points to as responsible? While a demo project would be appreciated, being able to examine the leaks you've seen would be helpful in the meantime.

Contributor

jshier commented Jan 30, 2017

Can you post the leaks output or the lines it points to as responsible? While a demo project would be appreciated, being able to examine the leaks you've seen would be helpful in the meantime.

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Jan 30, 2017

I have attached a couple of zipped CSV files. I have also confirmed the issue exists adding requests in a loop though I have no clear idea yet why. The following code block shows the problem code. It is running inside the getAllTasks completionHandler but has exactly the same issue running on the main queue.

IQSMS-Reporting-leaksWithLoopIssue.csv.zip
IQSMS-Reporting-leaksNoLoopIssue.csv.zip

unusuallyreticent commented Jan 30, 2017

I have attached a couple of zipped CSV files. I have also confirmed the issue exists adding requests in a loop though I have no clear idea yet why. The following code block shows the problem code. It is running inside the getAllTasks completionHandler but has exactly the same issue running on the main queue.

IQSMS-Reporting-leaksWithLoopIssue.csv.zip
IQSMS-Reporting-leaksNoLoopIssue.csv.zip

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Jan 30, 2017

OK, safari is not playing ball with my code when using // comments. The following code picks up unreadable folder files and starts a background download. The TransferID().string is a short encoded description string used to identify the task later. I'm not keeping any references to download tasks. Session.startsRequestsImmediately is false.

        if completed < total {
            let unreachable = folder.manifest.unreachable
            for rec in unreachable {
                let taskDescription = TransferID(type: .update, stagingId: stagingId, name: rec.filename, md5sum: rec.md5sum).string
                if taskDescriptions.contains(taskDescription) == false {
                    
                    /*
                    let req = self.session.download(rec.url)
                    req.task?.taskDescription = taskDescription
                    req.resume()
                     */
                    let req = self.session.session.downloadTask(with: rec.url)
                    req.taskDescription = taskDescription
                    req.resume()
                    
                }
                
            }
        } else if total > 0 {
            self.stagingID = 0
            _ = folder.convertToReady()
        }

unusuallyreticent commented Jan 30, 2017

OK, safari is not playing ball with my code when using // comments. The following code picks up unreadable folder files and starts a background download. The TransferID().string is a short encoded description string used to identify the task later. I'm not keeping any references to download tasks. Session.startsRequestsImmediately is false.

        if completed < total {
            let unreachable = folder.manifest.unreachable
            for rec in unreachable {
                let taskDescription = TransferID(type: .update, stagingId: stagingId, name: rec.filename, md5sum: rec.md5sum).string
                if taskDescriptions.contains(taskDescription) == false {
                    
                    /*
                    let req = self.session.download(rec.url)
                    req.task?.taskDescription = taskDescription
                    req.resume()
                     */
                    let req = self.session.session.downloadTask(with: rec.url)
                    req.taskDescription = taskDescription
                    req.resume()
                    
                }
                
            }
        } else if total > 0 {
            self.stagingID = 0
            _ = folder.convertToReady()
        }
@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Jan 30, 2017

There are 20 files in the code loop shown previously. I only have to loop for 4 to get the leaks trace. Each time the code run is repeated (deleting the folder of files), total memory used grows, so I'm beginning to think block is being retained somewhere.

unusuallyreticent commented Jan 30, 2017

There are 20 files in the code loop shown previously. I only have to loop for 4 to get the leaks trace. Each time the code run is repeated (deleting the folder of files), total memory used grows, so I'm beginning to think block is being retained somewhere.

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent commented Jan 30, 2017

screen shot 2017-01-30 at 04 41 31

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Jan 30, 2017

The loop code without the issue produces this image. I clicked refresh 10 times in the trace period. The error message on the App is because I faked an md5 checksum error on the last file in order to loop the test.
screen shot 2017-01-30 at 04 55 02

unusuallyreticent commented Jan 30, 2017

The loop code without the issue produces this image. I clicked refresh 10 times in the trace period. The error message on the App is because I faked an md5 checksum error on the last file in order to loop the test.
screen shot 2017-01-30 at 04 55 02

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Jan 30, 2017

Also, I forgot to mention, there are 2 x SessionManagers running. The default is doing the foreground fetches very nicely thank you :-). That should explain why there are 2 x NSLock objects shown.

unusuallyreticent commented Jan 30, 2017

Also, I forgot to mention, there are 2 x SessionManagers running. The default is doing the foreground fetches very nicely thank you :-). That should explain why there are 2 x NSLock objects shown.

@jshier

This comment has been minimized.

Show comment
Hide comment
@jshier

jshier Jan 30, 2017

Contributor

The lock leaks are a false positive. As for the background leaks, I'm not sure what's happening there, but it looks like you're leaking the world almost. If you can get a test project to us that would greatly help with our investigation.

Contributor

jshier commented Jan 30, 2017

The lock leaks are a false positive. As for the background leaks, I'm not sure what's happening there, but it looks like you're leaking the world almost. If you can get a test project to us that would greatly help with our investigation.

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Jan 30, 2017

I'll post a test later today.

unusuallyreticent commented Jan 30, 2017

I'll post a test later today.

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent commented Jan 30, 2017

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Jan 30, 2017

Do try setting the loop to 0..<4

unusuallyreticent commented Jan 30, 2017

Do try setting the loop to 0..<4

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Jan 31, 2017

Something is bit odd. In the test project SessionTest, unwire taskDidComplete. Note the function is empty in the test. Just wiring it up causes the issue in Instruments Leaks.

init() {
   // session.delegate.taskDidComplete = self.backgroundSessionTaskDidComplete
   session.delegate.downloadTaskDidFinishDownloadingToURL = self.downloadTaskDidFinishDownloadingToURL
}

unusuallyreticent commented Jan 31, 2017

Something is bit odd. In the test project SessionTest, unwire taskDidComplete. Note the function is empty in the test. Just wiring it up causes the issue in Instruments Leaks.

init() {
   // session.delegate.taskDidComplete = self.backgroundSessionTaskDidComplete
   session.delegate.downloadTaskDidFinishDownloadingToURL = self.downloadTaskDidFinishDownloadingToURL
}
@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Jan 31, 2017

Ok guys, bit of code for you to look at. The mod is not in the best place - should be in the if taskDidComplete block but I think you will get the idea as to what is going wrong if session.taskDidComplete is wired up:

open func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) {
    /// Executed after it is determined that the request is not going to be retried
    let completeTask: (URLSession, URLSessionTask, Error?) -> Void = { [weak self] session, task, error in
        guard let strongSelf = self else { return }

        if let taskDidComplete = strongSelf.taskDidComplete {
            taskDidComplete(session, task, error)
        } else if let delegate = strongSelf[task]?.delegate {
            delegate.urlSession(session, task: task, didCompleteWithError: error)
        }

        NotificationCenter.default.post(
            name: Notification.Name.Task.DidComplete,
            object: strongSelf,
            userInfo: [Notification.Key.Task: task]
        )
        
        // ADDED. delegate queue is not resumed if only Session.taskDidComplete is wired!
        if let delegate = strongSelf[task]?.delegate {
            delegate.queue.isSuspended = false
        }
        
        strongSelf[task] = nil
    }

    guard let request = self[task], let sessionManager = sessionManager else {
        completeTask(session, task, error)
        return
    }

    // Run all validations on the request before checking if an error occurred
    request.validations.forEach { $0() }

    // Determine whether an error has occurred
    var error: Error? = error

    if let taskDelegate = self[task]?.delegate, taskDelegate.error != nil {
        error = taskDelegate.error
    }

    /// If an error occurred and the retrier is set, asynchronously ask the retrier if the request
    /// should be retried. Otherwise, complete the task by notifying the task delegate.
    if let retrier = retrier, let error = error {
        retrier.should(sessionManager, retry: request, with: error) { [weak self] shouldRetry, timeDelay in
            guard shouldRetry else { completeTask(session, task, error) ; return }

            DispatchQueue.utility.after(timeDelay) { [weak self] in
                guard let strongSelf = self else { return }

                let retrySucceeded = strongSelf.sessionManager?.retry(request) ?? false

                if retrySucceeded, let task = request.task {
                    strongSelf[task] = request
                    return
                } else {
                    completeTask(session, task, error)
                }
            }
        }
    } else {
        completeTask(session, task, error)
    }
}

unusuallyreticent commented Jan 31, 2017

Ok guys, bit of code for you to look at. The mod is not in the best place - should be in the if taskDidComplete block but I think you will get the idea as to what is going wrong if session.taskDidComplete is wired up:

open func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) {
    /// Executed after it is determined that the request is not going to be retried
    let completeTask: (URLSession, URLSessionTask, Error?) -> Void = { [weak self] session, task, error in
        guard let strongSelf = self else { return }

        if let taskDidComplete = strongSelf.taskDidComplete {
            taskDidComplete(session, task, error)
        } else if let delegate = strongSelf[task]?.delegate {
            delegate.urlSession(session, task: task, didCompleteWithError: error)
        }

        NotificationCenter.default.post(
            name: Notification.Name.Task.DidComplete,
            object: strongSelf,
            userInfo: [Notification.Key.Task: task]
        )
        
        // ADDED. delegate queue is not resumed if only Session.taskDidComplete is wired!
        if let delegate = strongSelf[task]?.delegate {
            delegate.queue.isSuspended = false
        }
        
        strongSelf[task] = nil
    }

    guard let request = self[task], let sessionManager = sessionManager else {
        completeTask(session, task, error)
        return
    }

    // Run all validations on the request before checking if an error occurred
    request.validations.forEach { $0() }

    // Determine whether an error has occurred
    var error: Error? = error

    if let taskDelegate = self[task]?.delegate, taskDelegate.error != nil {
        error = taskDelegate.error
    }

    /// If an error occurred and the retrier is set, asynchronously ask the retrier if the request
    /// should be retried. Otherwise, complete the task by notifying the task delegate.
    if let retrier = retrier, let error = error {
        retrier.should(sessionManager, retry: request, with: error) { [weak self] shouldRetry, timeDelay in
            guard shouldRetry else { completeTask(session, task, error) ; return }

            DispatchQueue.utility.after(timeDelay) { [weak self] in
                guard let strongSelf = self else { return }

                let retrySucceeded = strongSelf.sessionManager?.retry(request) ?? false

                if retrySucceeded, let task = request.task {
                    strongSelf[task] = request
                    return
                } else {
                    completeTask(session, task, error)
                }
            }
        }
    } else {
        completeTask(session, task, error)
    }
}
@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Jan 31, 2017

Oh, I forgot to mention, if you just move the 'private let lock = NSLock()' right to the top of the SessionDelegate class, you want see this anymore in Leaks. It's a weird thing in Swift.

open class SessionDelegate: NSObject {
private let lock = NSLock() // <-- first in the class

unusuallyreticent commented Jan 31, 2017

Oh, I forgot to mention, if you just move the 'private let lock = NSLock()' right to the top of the SessionDelegate class, you want see this anymore in Leaks. It's a weird thing in Swift.

open class SessionDelegate: NSObject {
private let lock = NSLock() // <-- first in the class

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Jan 31, 2017

damn your spell checker! "you won't see ..."

unusuallyreticent commented Jan 31, 2017

damn your spell checker! "you won't see ..."

@chamira

This comment has been minimized.

Show comment
Hide comment
@chamira

chamira Feb 1, 2017

The issue looks different from simulator to device. I ran the same code both on device and the simulator they look different. (Leak object) But when I look at the stack trace on both SessionDelegate.init() can be seen.
screen shot 2017-02-01 at 10 45 02

screen shot 2017-02-01 at 10 45 09

@unusuallyreticent

Oh, I forgot to mention, if you just move the 'private let lock = NSLock()' right to the top of the SessionDelegate class, you want see this anymore in Leaks. It's a weird thing in Swift.

I tried it but nothing changed.

chamira commented Feb 1, 2017

The issue looks different from simulator to device. I ran the same code both on device and the simulator they look different. (Leak object) But when I look at the stack trace on both SessionDelegate.init() can be seen.
screen shot 2017-02-01 at 10 45 02

screen shot 2017-02-01 at 10 45 09

@unusuallyreticent

Oh, I forgot to mention, if you just move the 'private let lock = NSLock()' right to the top of the SessionDelegate class, you want see this anymore in Leaks. It's a weird thing in Swift.

I tried it but nothing changed.

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Feb 1, 2017

The Request adds a block operation referencing self, to the TaskDelegate.queue. I can't remember what it is off-hand, something to do with setting a time maybe. The taskDelegate is itself a strong property of the Request. The taskDelegate queue is not resumed in two situations. The first is when SessionDelegate.taskDidComplete is wired up. The second is when taskDelegate.TaskDidCompleteWithError is wired up. I am not 100% sure of the second case, however the first I can see clearly in debug. The queue is never resumed and the whole request and task delegate leak due to a strong reference cycle held inside that queue. At least, that's what I think it is. I not the guru here. I just make the tea.

The last cleanup point is inside the completeTask block inside the SessionDelegate didCompleteWithError function I listed earlier. The code modification I made ensures that queue is resumed, the queue operation completes and the reference cycle is remove:

if let delegate = strongSelf[task]?.delegate {
delegate.queue.isSuspended = false
}

Request.resume() has this, but task is NOT nil so it does not get executed in this case...
guard let task = task else { delegate.queue.isSuspended = false ; return }

The only other place where isSuspended is set false is in TaskDelegate didCompleteWithError. That bit of code is never run if SessionDelegate.taskDidComplete is wired up. If taskDidComplete is not wired up and TaskDelegate.taskDidCompleteWithError is wired up instead, then again, the queue is never resumed and the request (and all its references) and the task delegate leak.

The separate issue of the NSLock() appearing in instruments is I believe, due to a bit of a mess with swift initialisation. It seems to occur in static instance classes. If you want something really odd, try adding a couple of string dictionaries to the the AppDelegate. I guess it has something to do with the way Instruments works but, silly as it sounds, repositioning properties makes a difference. It can of course, work both ways. Repositioning can make something else appear to leak. Its just something I discovered by accident.

unusuallyreticent commented Feb 1, 2017

The Request adds a block operation referencing self, to the TaskDelegate.queue. I can't remember what it is off-hand, something to do with setting a time maybe. The taskDelegate is itself a strong property of the Request. The taskDelegate queue is not resumed in two situations. The first is when SessionDelegate.taskDidComplete is wired up. The second is when taskDelegate.TaskDidCompleteWithError is wired up. I am not 100% sure of the second case, however the first I can see clearly in debug. The queue is never resumed and the whole request and task delegate leak due to a strong reference cycle held inside that queue. At least, that's what I think it is. I not the guru here. I just make the tea.

The last cleanup point is inside the completeTask block inside the SessionDelegate didCompleteWithError function I listed earlier. The code modification I made ensures that queue is resumed, the queue operation completes and the reference cycle is remove:

if let delegate = strongSelf[task]?.delegate {
delegate.queue.isSuspended = false
}

Request.resume() has this, but task is NOT nil so it does not get executed in this case...
guard let task = task else { delegate.queue.isSuspended = false ; return }

The only other place where isSuspended is set false is in TaskDelegate didCompleteWithError. That bit of code is never run if SessionDelegate.taskDidComplete is wired up. If taskDidComplete is not wired up and TaskDelegate.taskDidCompleteWithError is wired up instead, then again, the queue is never resumed and the request (and all its references) and the task delegate leak.

The separate issue of the NSLock() appearing in instruments is I believe, due to a bit of a mess with swift initialisation. It seems to occur in static instance classes. If you want something really odd, try adding a couple of string dictionaries to the the AppDelegate. I guess it has something to do with the way Instruments works but, silly as it sounds, repositioning properties makes a difference. It can of course, work both ways. Repositioning can make something else appear to leak. Its just something I discovered by accident.

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Feb 1, 2017

AlamofireBackgroundSessionTest.zip

Here is the same test project with the SessionDelegate file modification in urlSession-didCompleteWithError, and the repositioned NSLock() just to make Instruments-leaks show pretty green lights.

unusuallyreticent commented Feb 1, 2017

AlamofireBackgroundSessionTest.zip

Here is the same test project with the SessionDelegate file modification in urlSession-didCompleteWithError, and the repositioned NSLock() just to make Instruments-leaks show pretty green lights.

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Feb 1, 2017

@chamira, the NSLock() property has to be first - before any other property. Look what I did to SessionDelegate in the test project. I only moved this single property to the top. You have moved retrier, sessionManager and requests as well. Leave them were they were. It's a really silly issue. Laughable really.

unusuallyreticent commented Feb 1, 2017

@chamira, the NSLock() property has to be first - before any other property. Look what I did to SessionDelegate in the test project. I only moved this single property to the top. You have moved retrier, sessionManager and requests as well. Leave them were they were. It's a really silly issue. Laughable really.

@jshier

This comment has been minimized.

Show comment
Hide comment
@jshier

jshier Feb 2, 2017

Contributor

@unusuallyreticent Thanks for the investigation, it helped narrow this down quite a bit. It turns out that, when the taskDidComplete closure is set, AF isn't calling it's own TaskDelegate, which means we lose all of the machinery in there that takes care of task completion. So not only would it leak but any response handlers attached would never be called. I've put up a tentative fix on the bug/1938 branch, pull it down with CocoaPods and see if it works for you. I still need to add some tests but it should fix the issues you're seeing.

Contributor

jshier commented Feb 2, 2017

@unusuallyreticent Thanks for the investigation, it helped narrow this down quite a bit. It turns out that, when the taskDidComplete closure is set, AF isn't calling it's own TaskDelegate, which means we lose all of the machinery in there that takes care of task completion. So not only would it leak but any response handlers attached would never be called. I've put up a tentative fix on the bug/1938 branch, pull it down with CocoaPods and see if it works for you. I still need to add some tests but it should fix the issues you're seeing.

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Feb 2, 2017

@jshier - thanks. I'll try it out. And, sorry if I seemed a bit rude occasionally. It was unintentional - late nights, three projects on the go, too much coffee :-(

I was only looking at my particular corner case - using taskDidComplete so I didn't consider that TaskDelegate should still be called anyway. Makes sense. They aren't exclusive.

Did you try that silly trick with NSLock()? That has got to be the daftest trick. If anyone told me to do that I would consider them loony.

unusuallyreticent commented Feb 2, 2017

@jshier - thanks. I'll try it out. And, sorry if I seemed a bit rude occasionally. It was unintentional - late nights, three projects on the go, too much coffee :-(

I was only looking at my particular corner case - using taskDidComplete so I didn't consider that TaskDelegate should still be called anyway. Makes sense. They aren't exclusive.

Did you try that silly trick with NSLock()? That has got to be the daftest trick. If anyone told me to do that I would consider them loony.

@unusuallyreticent

This comment has been minimized.

Show comment
Hide comment
@unusuallyreticent

unusuallyreticent Feb 2, 2017

@jshier - that's solved it.

unusuallyreticent commented Feb 2, 2017

@jshier - that's solved it.

@cnoon

This comment has been minimized.

Show comment
Hide comment
@cnoon

cnoon Feb 26, 2017

Member

Hey everyone,

Sorry for being late to the party here. @jshier and I discussed the changes that he put up in the bug/1938 branch. I'm not surprised that fixed the issue for you @unusuallyreticent. You have to be very careful right now with the session override closures to make sure you don't shut off some of the functionality.

The change that @jshier made is switching that taskDidComplete closure over to a hook instead of an override. After quite a bit of debate, we realized that there shouldn't be any harm in this change since it was impossible to get the queue resumed without a huge amount of work. Therefore we're fairly confident this change won't break any clients out there now.

We're going to look to unify the override closures in AF5 or just remove them completely with the goal being to make sure their usage is clear and not easy to mess up. There is enough inconsistency right now that it can lead to issues like you experienced.

As for the NSLock leak, I've actually filed a radar against that specific leak and it was closed as a duplicate. That is not something we can do anything about right now unfortunately.

I've pushed up @jshier's bug fix into the master branch in d64afca and it will go out as part of the Alamofire 4.4.0 release here shortly.

Cheers. 🍻

Member

cnoon commented Feb 26, 2017

Hey everyone,

Sorry for being late to the party here. @jshier and I discussed the changes that he put up in the bug/1938 branch. I'm not surprised that fixed the issue for you @unusuallyreticent. You have to be very careful right now with the session override closures to make sure you don't shut off some of the functionality.

The change that @jshier made is switching that taskDidComplete closure over to a hook instead of an override. After quite a bit of debate, we realized that there shouldn't be any harm in this change since it was impossible to get the queue resumed without a huge amount of work. Therefore we're fairly confident this change won't break any clients out there now.

We're going to look to unify the override closures in AF5 or just remove them completely with the goal being to make sure their usage is clear and not easy to mess up. There is enough inconsistency right now that it can lead to issues like you experienced.

As for the NSLock leak, I've actually filed a radar against that specific leak and it was closed as a duplicate. That is not something we can do anything about right now unfortunately.

I've pushed up @jshier's bug fix into the master branch in d64afca and it will go out as part of the Alamofire 4.4.0 release here shortly.

Cheers. 🍻

@cnoon cnoon closed this Feb 26, 2017

@cnoon cnoon added this to the 4.4.0 milestone Feb 26, 2017

looseyi added a commit to looseyi/Alamofire that referenced this issue May 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment