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 a crash when several request for the same URL or route are launched together #399

Merged
merged 2 commits into from
May 1, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
version: 2

jobs:
danger:

Danger:
macos:
xcode: 10.2.0
steps:
Expand All @@ -24,7 +25,8 @@ jobs:
- run:
name: Danger
command: bundle exec danger
macos:

Test OS X Platform:
environment:
TEST_REPORTS: /tmp/test-results
LANG: en_US.UTF-8
Expand Down Expand Up @@ -58,14 +60,11 @@ jobs:
- store_artifacts:
path: /tmp/test-results

linux:
Test Linux Platform:
docker:
- image: swift:4.2
- image: swift:5.0
steps:
- checkout
- run:
name: Compile code
command: swift build
- run:
name: Run Unit Tests
command: swift test
Expand All @@ -74,10 +73,10 @@ workflows:
version: 2
tests:
jobs:
- danger
- linux:
- Danger
- Test Linux Platform:
requires:
- danger
- macos:
- Danger
- Test OS X Platform:
requires:
- danger
- Danger
1 change: 1 addition & 0 deletions .swiftlint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ disabled_rules:

excluded: # paths to ignore during linting. Takes precedence over `included`.
- LinuxMain.swift
- XCode/Tests/XCTestManifests.swift
- Tests/XCTestManifests.swift
- Package.swift
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ All notable changes to this project will be documented in this file. Changes not
- Added [Danger](https://danger.systems/ruby/) and Swiftlint to the project. ([#398](https://github.com/httpswift/swifter/pull/398)) by [@Vkt0r](https://github.com/Vkt0r)

## Fixed
- An issue causing a crash regarding a thread race condition. ([#399](https://github.com/httpswift/swifter/pull/399)) by [@Vkt0r](https://github.com/Vkt0r)
- An issue in the `HttpRouter` causing issues to handle routes with overlapping in the tail. ([#379](https://github.com/httpswift/swifter/pull/359), [#382](https://github.com/httpswift/swifter/pull/382)) by [@Vkt0r](https://github.com/Vkt0r)

- Fixes build errors by excluding XC(UI)Test files from regular targets [#397](https://github.com/httpswift/swifter/pull/397)) by [@ChristianSteffens](https://github.com/ChristianSteffens)
Expand Down
25 changes: 21 additions & 4 deletions Package.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version:4.0
// swift-tools-version:5.0

import PackageDescription

Expand All @@ -13,8 +13,25 @@ let package = Package(
dependencies: [],

targets: [
.target(name: "Swifter", dependencies: [], path: "XCode/Sources"),
.target(name: "Example", dependencies: ["Swifter"], path: "Example"),
.testTarget(name: "SwifterTests", dependencies: ["Swifter"], path: "XCode/Tests")
.target(
name: "Swifter",
dependencies: [],
path: "XCode/Sources"
),

.target(
name: "Example",
dependencies: [
"Swifter"
],
path: "Example"),

.testTarget(
name: "SwifterTests",
dependencies: [
"Swifter"
],
path: "XCode/Tests"
)
]
)
26 changes: 17 additions & 9 deletions XCode/Sources/HttpRouter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ open class HttpRouter {
}

private var rootNode = Node()

/// The Queue to handle the thread safe access to the routes
private let queue = DispatchQueue(label: "swifter.httpserverio.httprouter")

public func routes() -> [String] {
var routes = [String]()
Expand Down Expand Up @@ -56,21 +59,26 @@ open class HttpRouter {
}

public func route(_ method: String?, path: String) -> ([String: String], (HttpRequest) -> HttpResponse)? {
if let method = method {
let pathSegments = (method + "/" + stripQuery(path)).split("/")

return queue.sync {
if let method = method {
let pathSegments = (method + "/" + stripQuery(path)).split("/")
var pathSegmentsGenerator = pathSegments.makeIterator()
var params = [String: String]()
if let handler = findHandler(&rootNode, params: &params, generator: &pathSegmentsGenerator) {
return (params, handler)
}
}

let pathSegments = ("*/" + stripQuery(path)).split("/")
var pathSegmentsGenerator = pathSegments.makeIterator()
var params = [String: String]()
if let handler = findHandler(&rootNode, params: &params, generator: &pathSegmentsGenerator) {
return (params, handler)
}

return nil
}
let pathSegments = ("*/" + stripQuery(path)).split("/")
var pathSegmentsGenerator = pathSegments.makeIterator()
var params = [String: String]()
if let handler = findHandler(&rootNode, params: &params, generator: &pathSegmentsGenerator) {
return (params, handler)
}
return nil
}

private func inflate(_ node: inout Node, generator: inout IndexingIterator<[String]>) -> Node {
Expand Down
2 changes: 2 additions & 0 deletions XCode/Sources/HttpServerIO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ public class HttpServerIO {
strongSelf.queue.async {
strongSelf.sockets.insert(socket)
}

strongSelf.handleConnection(socket)

strongSelf.queue.async {
strongSelf.sockets.remove(socket)
}
Expand Down
13 changes: 10 additions & 3 deletions XCode/Swifter.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
269B47981D3AAAE20042D137 /* Errno.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C76B2A11D369C9D00D35BFB /* Errno.swift */; };
269B47991D3AAAE20042D137 /* String+BASE64.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C76B6F61D2C44F30030FC98 /* String+BASE64.swift */; };
269B47A71D3AAC4F0042D137 /* SwiftertvOS.h in Headers */ = {isa = PBXBuildFile; fileRef = 269B47A51D3AAC4F0042D137 /* SwiftertvOS.h */; settings = {ATTRIBUTES = (Public, ); }; };
542604AE226A33540065B874 /* XCTestManifests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B912F49220507D00062C360 /* XCTestManifests.swift */; };
6A0D4512204E9988000A0726 /* MimeTypesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6A0D4511204E9988000A0726 /* MimeTypesTests.swift */; };
6AE2FF722048013000302EC4 /* MimeTypes.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6AE2FF702048011A00302EC4 /* MimeTypes.swift */; };
6AE2FF732048013000302EC4 /* MimeTypes.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6AE2FF702048011A00302EC4 /* MimeTypes.swift */; };
Expand All @@ -55,6 +54,9 @@
7AE893FE1C0512C400A29F63 /* SwifterMac.h in Headers */ = {isa = PBXBuildFile; fileRef = 7AE893FD1C0512C400A29F63 /* SwifterMac.h */; settings = {ATTRIBUTES = (Public, ); }; };
7AE8940D1C05151100A29F63 /* Launch Screen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 7AE8940C1C05151100A29F63 /* Launch Screen.storyboard */; };
7B11AD4B21C9A8A6002F8820 /* SwifterTestsHttpRouter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7CCB8C5F1D97B8CC008B9712 /* SwifterTestsHttpRouter.swift */; };
7B55EC95226E0E4F00042D23 /* ServerThreadingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B55EC94226E0E4F00042D23 /* ServerThreadingTests.swift */; };
7B55EC96226E0E4F00042D23 /* ServerThreadingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B55EC94226E0E4F00042D23 /* ServerThreadingTests.swift */; };
7B55EC97226E0E4F00042D23 /* ServerThreadingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B55EC94226E0E4F00042D23 /* ServerThreadingTests.swift */; };
7B74CFA82163C40F001BE07B /* AppDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7CDAB80C1BE2A1D400C8A977 /* AppDelegate.swift */; };
7C377E181D964B96009C6148 /* String+File.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C377E161D964B6A009C6148 /* String+File.swift */; };
7C377E191D964B9F009C6148 /* String+File.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C377E161D964B6A009C6148 /* String+File.swift */; };
Expand Down Expand Up @@ -177,6 +179,7 @@
7AE893FD1C0512C400A29F63 /* SwifterMac.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = SwifterMac.h; sourceTree = "<group>"; };
7AE893FF1C0512C400A29F63 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
7AE8940C1C05151100A29F63 /* Launch Screen.storyboard */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.storyboard; path = "Launch Screen.storyboard"; sourceTree = "<group>"; };
7B55EC94226E0E4F00042D23 /* ServerThreadingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ServerThreadingTests.swift; sourceTree = "<group>"; };
7B912F49220507D00062C360 /* XCTestManifests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = XCTestManifests.swift; sourceTree = "<group>"; };
7B912F4B220507DB0062C360 /* LinuxMain.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LinuxMain.swift; sourceTree = "<group>"; };
7C377E161D964B6A009C6148 /* String+File.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "String+File.swift"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -417,6 +420,7 @@
7C4785E81C71D15600A9FE73 /* SwifterTestsWebSocketSession.swift */,
0858E7F31D68BB2600491CD1 /* IOSafetyTests.swift */,
6A0D4511204E9988000A0726 /* MimeTypesTests.swift */,
7B55EC94226E0E4F00042D23 /* ServerThreadingTests.swift */,
);
path = Tests;
sourceTree = "<group>";
Expand Down Expand Up @@ -769,6 +773,7 @@
047F1F02226AB9AD00909B95 /* XCTestManifests.swift in Sources */,
043660CE21FED35500497989 /* SwifterTestsHttpParser.swift in Sources */,
043660D521FED36C00497989 /* MimeTypesTests.swift in Sources */,
7B55EC96226E0E4F00042D23 /* ServerThreadingTests.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand All @@ -781,6 +786,7 @@
043660EA21FED51E00497989 /* IOSafetyTests.swift in Sources */,
043660E821FED51900497989 /* SwifterTestsStringExtensions.swift in Sources */,
043660E521FED51100497989 /* SwifterTestsHttpRouter.swift in Sources */,
7B55EC97226E0E4F00042D23 /* ServerThreadingTests.swift in Sources */,
043660E621FED51400497989 /* SwifterTestsHttpParser.swift in Sources */,
043660EB21FED52000497989 /* MimeTypesTests.swift in Sources */,
);
Expand Down Expand Up @@ -896,6 +902,7 @@
043660D421FED36900497989 /* IOSafetyTests.swift in Sources */,
7CCD87721C660B250068099B /* SwifterTestsStringExtensions.swift in Sources */,
7B11AD4B21C9A8A6002F8820 /* SwifterTestsHttpRouter.swift in Sources */,
7B55EC95226E0E4F00042D23 /* ServerThreadingTests.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down Expand Up @@ -1278,7 +1285,7 @@
OTHER_SWIFT_FLAGS = "-Xfrontend -warn-long-function-bodies=500";
SDKROOT = macosx;
SWIFT_OPTIMIZATION_LEVEL = "-Onone";
SWIFT_VERSION = 4.2;
SWIFT_VERSION = 5.0;
TARGETED_DEVICE_FAMILY = "1,2";
};
name = Debug;
Expand Down Expand Up @@ -1333,7 +1340,7 @@
OTHER_SWIFT_FLAGS = "-Xfrontend -warn-long-function-bodies=500";
SDKROOT = macosx;
SWIFT_OPTIMIZATION_LEVEL = "-Owholemodule";
SWIFT_VERSION = 4.2;
SWIFT_VERSION = 5.0;
TARGETED_DEVICE_FAMILY = "1,2";
VALIDATE_PRODUCT = YES;
};
Expand Down
10 changes: 8 additions & 2 deletions XCode/Tests/IOSafetyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,22 @@ import XCTest

class IOSafetyTests: XCTestCase {
var server: HttpServer!
var urlSession: URLSession!

override func setUp() {
super.setUp()
server = HttpServer.pingServer()
urlSession = URLSession(configuration: .default)
}

override func tearDown() {
if server.operating {
server.stop()
}

urlSession = nil
server = nil

super.tearDown()
}

Expand All @@ -29,10 +35,10 @@ class IOSafetyTests: XCTestCase {
server = HttpServer.pingServer()
do {
try server.start()
XCTAssertFalse(URLSession.shared.retryPing())
XCTAssertFalse(urlSession.retryPing())
(0...100).forEach { _ in
DispatchQueue.global(qos: DispatchQoS.default.qosClass).sync {
URLSession.shared.pingTask { _, _, _ in }.resume()
urlSession.pingTask { _, _, _ in }.resume()
}
}
server.stop()
Expand Down
115 changes: 115 additions & 0 deletions XCode/Tests/ServerThreadingTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
//
// ServerThreadingTests.swift
// Swifter
//
// Created by Victor Sigler on 4/22/19.
// Copyright © 2019 Damian Kołakowski. All rights reserved.
//

import XCTest
@testable import Swifter

class ServerThreadingTests: XCTestCase {

var server: HttpServer!

override func setUp() {
super.setUp()
server = HttpServer()
}

override func tearDown() {
if server.operating {
server.stop()
}
server = nil
super.tearDown()
}

func testShouldHandleTheSameRequestWithDifferentTimeIntervals() {

let path = "/a/:b/c"
let queue = DispatchQueue(label: "com.swifter.threading")
let hostURL: URL

server.GET[path] = { .ok(.html("You asked for " + $0.path)) }

do {

#if os(Linux)
try server.start(9081)
hostURL = URL(string: "http://localhost:9081")!
#else
try server.start()
hostURL = defaultLocalhost
#endif

let requestExpectation = expectation(description: "Request should finish.")
requestExpectation.expectedFulfillmentCount = 3

(1...3).forEach { index in
queue.asyncAfter(deadline: .now() + .seconds(index)) {
let task = URLSession.shared.executeAsyncTask(hostURL: hostURL, path: path) { (_, response, _ ) in
requestExpectation.fulfill()
let statusCode = (response as? HTTPURLResponse)?.statusCode
XCTAssertNotNil(statusCode)
XCTAssertEqual(statusCode, 200, "\(hostURL)")
}

task.resume()
}
}

} catch let error {
XCTFail("\(error)")
}

waitForExpectations(timeout: 10, handler: nil)
}

func testShouldHandleTheSameRequestConcurrently() {

let path = "/a/:b/c"
server.GET[path] = { .ok(.html("You asked for " + $0.path)) }

var requestExpectation: XCTestExpectation? = expectation(description: "Should handle the request concurrently")

do {

try server.start()
let downloadGroup = DispatchGroup()

DispatchQueue.concurrentPerform(iterations: 3) { _ in
downloadGroup.enter()

let task = URLSession.shared.executeAsyncTask(path: path) { (_, response, _ ) in

let statusCode = (response as? HTTPURLResponse)?.statusCode
XCTAssertNotNil(statusCode)
XCTAssertEqual(statusCode, 200)
requestExpectation?.fulfill()
requestExpectation = nil
downloadGroup.leave()
}

task.resume()
}

} catch let error {
XCTFail("\(error)")
}

waitForExpectations(timeout: 15, handler: nil)
}
}

extension URLSession {

func executeAsyncTask(
hostURL: URL = defaultLocalhost,
path: String,
completionHandler handler: @escaping (Data?, URLResponse?, Error?) -> Void
) -> URLSessionDataTask {
return self.dataTask(with: hostURL.appendingPathComponent(path), completionHandler: handler)
}
}
Loading