Skip to content
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

Fix memory leaks #3199

Merged
merged 7 commits into from May 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 10 additions & 10 deletions .github/workflows/ci.yml
Expand Up @@ -17,22 +17,22 @@ jobs:
DEVELOPER_DIR: /Applications/Xcode_11.3.1.app/Contents/Developer
steps:
- uses: actions/checkout@v2
- name: macOS
- name: macOS (5.1)
run: set -o pipefail && env NSUnbufferedIO=YES xcodebuild -project "Alamofire.xcodeproj" -scheme "Alamofire macOS" -destination "platform=macOS" clean test | xcpretty
macOS_5_2:
name: Test macOS (5.2)
runs-on: macOS-latest
env:
DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
DEVELOPER_DIR: /Applications/Xcode_11.5.app/Contents/Developer
steps:
- uses: actions/checkout@v2
- name: macOS
- name: macOS (5.2)
run: set -o pipefail && env NSUnbufferedIO=YES xcodebuild -project "Alamofire.xcodeproj" -scheme "Alamofire macOS" -destination "platform=macOS" clean test | xcpretty
Catalyst:
name: Test Catalyst
runs-on: macOS-latest
env:
DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
DEVELOPER_DIR: /Applications/Xcode_11.5.app/Contents/Developer
steps:
- uses: actions/checkout@v2
- name: Catalyst
Expand All @@ -41,10 +41,10 @@ jobs:
name: Test iOS
runs-on: macOS-latest
env:
DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
DEVELOPER_DIR: /Applications/Xcode_11.5.app/Contents/Developer
strategy:
matrix:
destination: ["OS=13.4.1,name=iPhone 11 Pro"] #, "OS=12.4,name=iPhone XS", "OS=11.4,name=iPhone X", "OS=10.3.1,name=iPhone SE"]
destination: ["OS=13.5,name=iPhone 11 Pro"] #, "OS=12.4,name=iPhone XS", "OS=11.4,name=iPhone X", "OS=10.3.1,name=iPhone SE"]
steps:
- uses: actions/checkout@v2
- name: iOS - ${{ matrix.destination }}
Expand All @@ -53,7 +53,7 @@ jobs:
name: Test tvOS
runs-on: macOS-latest
env:
DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
DEVELOPER_DIR: /Applications/Xcode_11.5.app/Contents/Developer
strategy:
matrix:
destination: ["OS=13.4,name=Apple TV 4K"] #, "OS=11.4,name=Apple TV 4K", "OS=10.2,name=Apple TV 1080p"]
Expand All @@ -65,10 +65,10 @@ jobs:
name: Build watchOS
runs-on: macOS-latest
env:
DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
DEVELOPER_DIR: /Applications/Xcode_11.5.app/Contents/Developer
strategy:
matrix:
destination: ["OS=6.2,name=Apple Watch Series 5 - 44mm"] #, "OS=4.2,name=Apple Watch Series 3 - 42mm", "OS=3.2,name=Apple Watch Series 2 - 42mm"]
destination: ["OS=6.2.1,name=Apple Watch Series 5 - 44mm"] #, "OS=4.2,name=Apple Watch Series 3 - 42mm", "OS=3.2,name=Apple Watch Series 2 - 42mm"]
steps:
- uses: actions/checkout@v2
- name: watchOS - ${{ matrix.destination }}
Expand All @@ -77,7 +77,7 @@ jobs:
name: Test with SPM
runs-on: macOS-latest
env:
DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
DEVELOPER_DIR: /Applications/Xcode_11.5.app/Contents/Developer
steps:
- uses: actions/checkout@v2
- name: SPM Test
Expand Down
8 changes: 8 additions & 0 deletions Alamofire.xcodeproj/project.pbxproj
Expand Up @@ -78,6 +78,9 @@
31727422218BB9A50039FFCC /* HTTPBin.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31727421218BB9A50039FFCC /* HTTPBin.swift */; };
31727423218BB9A50039FFCC /* HTTPBin.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31727421218BB9A50039FFCC /* HTTPBin.swift */; };
31727424218BB9A50039FFCC /* HTTPBin.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31727421218BB9A50039FFCC /* HTTPBin.swift */; };
31762DCA247738FA0025C704 /* LeaksTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31762DC9247738FA0025C704 /* LeaksTests.swift */; };
31762DCB247738FA0025C704 /* LeaksTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31762DC9247738FA0025C704 /* LeaksTests.swift */; };
31762DCC247738FA0025C704 /* LeaksTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31762DC9247738FA0025C704 /* LeaksTests.swift */; };
317A6A7620B2207F00A9FEC5 /* DownloadTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F8111E5B19A9674D0040E7D1 /* DownloadTests.swift */; };
317A6A7720B2208000A9FEC5 /* DownloadTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F8111E5B19A9674D0040E7D1 /* DownloadTests.swift */; };
317A6A7820B2208000A9FEC5 /* DownloadTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F8111E5B19A9674D0040E7D1 /* DownloadTests.swift */; };
Expand Down Expand Up @@ -391,6 +394,7 @@
31727417218BAEC90039FFCC /* HTTPMethod.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HTTPMethod.swift; sourceTree = "<group>"; };
3172741C218BB1790039FFCC /* ParameterEncoder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ParameterEncoder.swift; sourceTree = "<group>"; };
31727421218BB9A50039FFCC /* HTTPBin.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HTTPBin.swift; sourceTree = "<group>"; };
31762DC9247738FA0025C704 /* LeaksTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LeaksTests.swift; sourceTree = "<group>"; };
318DD40E2439780500963291 /* Combine.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Combine.swift; sourceTree = "<group>"; };
3191B5741F5F53A6003960A8 /* Protected.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Protected.swift; sourceTree = "<group>"; };
31991790209CDA7F00103A19 /* Request.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Request.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -907,6 +911,7 @@
children = (
4C256A501B096C2C0065714F /* BaseTestCase.swift */,
31727421218BB9A50039FFCC /* HTTPBin.swift */,
31762DC9247738FA0025C704 /* LeaksTests.swift */,
31F9683B20BB70290009606F /* NSLoggingEventMonitor.swift */,
4C256A4E1B09656A0065714F /* Core */,
4C7C8D201B9D0D7300948136 /* Extensions */,
Expand Down Expand Up @@ -1379,6 +1384,7 @@
31C2B0EC20B271060089BA7C /* CacheTests.swift in Sources */,
3111CE9120A7EC27008315E2 /* NetworkReachabilityManagerTests.swift in Sources */,
31BADE502439A8D1007D2AB9 /* CombineTests.swift in Sources */,
31762DCC247738FA0025C704 /* LeaksTests.swift in Sources */,
31425AC3241F098000EE3CCC /* InternalRequestTests.swift in Sources */,
4CF627171BA7CC240011A099 /* ParameterEncodingTests.swift in Sources */,
);
Expand Down Expand Up @@ -1546,6 +1552,7 @@
31C2B0EA20B271040089BA7C /* CacheTests.swift in Sources */,
3111CE8F20A7EC26008315E2 /* NetworkReachabilityManagerTests.swift in Sources */,
31BADE4E2439A8D1007D2AB9 /* CombineTests.swift in Sources */,
31762DCA247738FA0025C704 /* LeaksTests.swift in Sources */,
31425AC1241F098000EE3CCC /* InternalRequestTests.swift in Sources */,
F8111E6119A9674D0040E7D1 /* ParameterEncodingTests.swift in Sources */,
);
Expand Down Expand Up @@ -1587,6 +1594,7 @@
31C2B0EB20B271050089BA7C /* CacheTests.swift in Sources */,
3111CE9020A7EC27008315E2 /* NetworkReachabilityManagerTests.swift in Sources */,
31BADE4F2439A8D1007D2AB9 /* CombineTests.swift in Sources */,
31762DCB247738FA0025C704 /* LeaksTests.swift in Sources */,
31425AC2241F098000EE3CCC /* InternalRequestTests.swift in Sources */,
4C256A541B096C770065714F /* BaseTestCase.swift in Sources */,
);
Expand Down
4 changes: 3 additions & 1 deletion Source/Request.swift
Expand Up @@ -1599,7 +1599,9 @@ public class DownloadRequest: Request {

let result = validation(self.request, response, self.fileURL)

if case let .failure(error) = result { self.error = error.asAFError(or: .responseValidationFailed(reason: .customValidationFailed(error: error))) }
if case let .failure(error) = result {
self.error = error.asAFError(or: .responseValidationFailed(reason: .customValidationFailed(error: error)))
}

self.eventMonitor?.request(self,
didValidateRequest: self.request,
Expand Down
6 changes: 3 additions & 3 deletions Source/ResponseSerialization.swift
Expand Up @@ -952,7 +952,7 @@ extension DataStreamRequest {
/// - Returns: The `DataStreamRequest`.
@discardableResult
public func responseStream(on queue: DispatchQueue = .main, stream: @escaping Handler<Data, Never>) -> Self {
$streamMutableState.write { state in
$streamMutableState.write { [unowned self] state in
let capture = (queue, { data in
self.capturingError {
try stream(.init(event: .stream(.success(data)), token: .init(self)))
Expand All @@ -977,7 +977,7 @@ extension DataStreamRequest {
public func responseStream<Serializer: DataStreamSerializer>(using serializer: Serializer,
on queue: DispatchQueue = .main,
stream: @escaping Handler<Serializer.SerializedObject, AFError>) -> Self {
let parser = { (data: Data) in
let parser = { [unowned self] (data: Data) in
self.serializationQueue.async {
// Start work on serialization queue.
let result = Result { try serializer.serialize(data) }
Expand Down Expand Up @@ -1011,7 +1011,7 @@ extension DataStreamRequest {
@discardableResult
public func responseStreamString(on queue: DispatchQueue = .main,
stream: @escaping Handler<String, Never>) -> Self {
let parser = { (data: Data) in
let parser = { [unowned self] (data: Data) in
self.serializationQueue.async {
// Start work on serialization queue.
let string = String(decoding: data, as: UTF8.self)
Expand Down
5 changes: 4 additions & 1 deletion Source/Validation.swift
Expand Up @@ -232,7 +232,10 @@ extension DataStreamRequest {
/// - Returns: The instance.
@discardableResult
public func validate() -> Self {
validate(statusCode: acceptableStatusCodes).validate(contentType: self.acceptableContentTypes)
let contentTypes: () -> [String] = { [unowned self] in
self.acceptableContentTypes
}
return validate(statusCode: acceptableStatusCodes).validate(contentType: contentTypes())
}
}

Expand Down
16 changes: 8 additions & 8 deletions Tests/CombineTests.swift
Expand Up @@ -863,21 +863,21 @@ final class DataStreamRequestCombineTests: CombineTestCase {
// Given
let responseReceived = expectation(description: "response should be received")
let completionReceived = expectation(description: "stream should complete")
var stream: DataStreamRequest.Stream<HTTPBinResponse, AFError>?
var error: AFError?

// When
let request = AF.streamRequest(URLRequest.makeHTTPBinRequest())
var token: AnyCancellable? = request
.publishDecodable(type: HTTPBinResponse.self)
.sink(receiveCompletion: { _ in completionReceived.fulfill() },
receiveValue: { stream = $0; responseReceived.fulfill() })
receiveValue: { error = $0.completion?.error; responseReceived.fulfill() })
token = nil

waitForExpectations(timeout: timeout)

// Then
XCTAssertNotNil(stream?.completion?.error)
XCTAssertTrue(stream?.completion?.error?.isExplicitlyCancelledError == true)
XCTAssertNotNil(error)
XCTAssertTrue(error?.isExplicitlyCancelledError == true)
XCTAssertTrue(request.isCancelled)
XCTAssertNil(token)
}
Expand All @@ -887,23 +887,23 @@ final class DataStreamRequestCombineTests: CombineTestCase {
// Given
let responseReceived = expectation(description: "response should be received")
let completionReceived = expectation(description: "stream should complete")
var stream: DataStreamRequest.Stream<HTTPBinResponse, AFError>?
var error: AFError?

// When
let request = AF.streamRequest(URLRequest.makeHTTPBinRequest())
store {
request
.publishDecodable(type: HTTPBinResponse.self)
.sink(receiveCompletion: { _ in completionReceived.fulfill() },
receiveValue: { stream = $0; responseReceived.fulfill() })
receiveValue: { error = $0.completion?.error; responseReceived.fulfill() })
}
request.cancel()

waitForExpectations(timeout: timeout)

// Then
XCTAssertNotNil(stream?.completion?.error)
XCTAssertTrue(stream?.completion?.error?.isExplicitlyCancelledError == true)
XCTAssertNotNil(error)
XCTAssertTrue(error?.isExplicitlyCancelledError == true)
XCTAssertTrue(request.isCancelled)
}

Expand Down
12 changes: 6 additions & 6 deletions Tests/DownloadTests.swift
Expand Up @@ -459,7 +459,7 @@ final class DownloadRequestEventsTestCase: BaseTestCase {
responseHandler.fulfill()
}

eventMonitor.requestDidResumeTask = { _, _ in
eventMonitor.requestDidResumeTask = { [unowned request] _, _ in
request.cancel()
didResumeTask.fulfill()
}
Expand Down Expand Up @@ -487,7 +487,7 @@ final class DownloadResumeDataTestCase: BaseTestCase {

// When
let download = AF.download(urlString)
download.downloadProgress { progress in
download.downloadProgress { [unowned download] progress in
guard !cancelled else { return }

if progress.fractionCompleted > 0.1 {
Expand Down Expand Up @@ -521,7 +521,7 @@ final class DownloadResumeDataTestCase: BaseTestCase {

// When
let download = AF.download(urlString)
download.downloadProgress { progress in
download.downloadProgress { [unowned download] progress in
guard !cancelled else { return }

if progress.fractionCompleted > 0.1 {
Expand Down Expand Up @@ -557,7 +557,7 @@ final class DownloadResumeDataTestCase: BaseTestCase {

// When
let download = AF.download(urlString)
download.downloadProgress { progress in
download.downloadProgress { [unowned download] progress in
guard !cancelled else { return }

if progress.fractionCompleted > 0.1 {
Expand Down Expand Up @@ -594,7 +594,7 @@ final class DownloadResumeDataTestCase: BaseTestCase {

// When
let download = AF.download(urlString)
download.downloadProgress { progress in
download.downloadProgress { [unowned download] progress in
guard !cancelled else { return }

if progress.fractionCompleted > 0.1 {
Expand Down Expand Up @@ -654,7 +654,7 @@ final class DownloadResumeDataTestCase: BaseTestCase {

// When
let download = AF.download(urlString)
download.downloadProgress { progress in
download.downloadProgress { [unowned download] progress in
guard !cancelled else { return }

if progress.fractionCompleted > 0.1 {
Expand Down
69 changes: 69 additions & 0 deletions Tests/LeaksTests.swift
@@ -0,0 +1,69 @@
//
// LeaksTests.swift
//
// Copyright (c) 2020 Alamofire Software Foundation (http://alamofire.org/)
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
//

import XCTest

// Only build when built through SPM, as tests run through Xcode don't like this.
// Add LEAKS flag once we figure out a way to automate this.
// Can run by invoking swift test -c debug -Xswiftc -DLEAKS in the Alamofire directory.
// Sample code from the Swift forums: https://forums.swift.org/t/test-for-memory-leaks-in-ci/36526/19
#if SWIFT_PACKAGE && LEAKS && os(macOS)
final class LeaksTests: XCTestCase {
func testForLeaks() {
// Sets up an atexit handler that invokes the leaks tool.
atexit {
@discardableResult
func leaksTo(_ file: String) -> Process {
let out = FileHandle(forWritingAtPath: file)!
defer {
if #available(OSX 10.15, *) {
try! out.close()
} else {
// Fallback on earlier versions
}
}
let process = Process()
process.launchPath = "/usr/bin/leaks"
process.arguments = ["\(getpid())"]
process.standardOutput = out
process.standardError = out
process.launch()
process.waitUntilExit()
return process
}
let process = leaksTo("/dev/null")
guard process.terminationReason == .exit && [0, 1].contains(process.terminationStatus) else {
print("Process terminated: \(process.terminationReason): \(process.terminationStatus)")
exit(255)
}
if process.terminationStatus == 1 {
print("================")
print("Leaks Detected!!!")
leaksTo("/dev/tty")
}
exit(process.terminationStatus)
}
}
}
#endif
8 changes: 4 additions & 4 deletions Tests/RequestTests.swift
Expand Up @@ -490,7 +490,7 @@ final class RequestResponseTestCase: BaseTestCase {
// When
let request = session.request(URLRequest.makeHTTPBinRequest())
// Cancellation stops task creation, so don't cancel the request until the task has been created.
eventMonitor.requestDidCreateTask = { _, _ in
eventMonitor.requestDidCreateTask = { [unowned request] _, _ in
for _ in 0..<100 {
request.cancel()
}
Expand Down Expand Up @@ -521,7 +521,7 @@ final class RequestResponseTestCase: BaseTestCase {
// When
let request = session.request(URLRequest.makeHTTPBinRequest())
// Cancellation stops task creation, so don't cancel the request until the task has been created.
eventMonitor.requestDidCreateTask = { _, _ in
eventMonitor.requestDidCreateTask = { [unowned request] _, _ in
for _ in 0..<100 {
request.cancel()
}
Expand Down Expand Up @@ -552,7 +552,7 @@ final class RequestResponseTestCase: BaseTestCase {
// When
let request = session.request(URLRequest.makeHTTPBinRequest(path: "delay/5")).response { _ in expect.fulfill() }
// Cancellation stops task creation, so don't cancel the request until the task has been created.
eventMonitor.requestDidCreateTask = { _, _ in
eventMonitor.requestDidCreateTask = { [unowned request] _, _ in
DispatchQueue.concurrentPerform(iterations: 100) { i in
request.cancel()

Expand Down Expand Up @@ -650,7 +650,7 @@ final class RequestResponseTestCase: BaseTestCase {
responseHandler.fulfill()
}

eventMonitor.requestDidResumeTask = { _, _ in
eventMonitor.requestDidResumeTask = { [unowned request] _, _ in
request.cancel()
didResumeTask.fulfill()
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/SessionTests.swift
Expand Up @@ -1589,7 +1589,7 @@ final class SessionCancellationTestCase: BaseTestCase {
let createTask = expectation(description: "should create task twice")
createTask.expectedFulfillmentCount = 2
var tasksCreated = 0
monitor.requestDidCreateTask = { _, _ in
monitor.requestDidCreateTask = { [unowned session] _, _ in
tasksCreated += 1
createTask.fulfill()
// Cancel after the second task is created to ensure proper lifetime events.
Expand Down