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

[SR-13383] Windows, Dispatch crashes after network request timeout #609

Open
lxbndr opened this issue Aug 12, 2020 · 5 comments
Open

[SR-13383] Windows, Dispatch crashes after network request timeout #609

lxbndr opened this issue Aug 12, 2020 · 5 comments

Comments

@lxbndr
Copy link

lxbndr commented Aug 12, 2020

Previous ID SR-13383
Radar None
Original Reporter @lxbndr
Type Bug

Attachment: Download

Environment

Swift version 5.3-dev (LLVM 8cbcb68709, Swift df007a4f27)

Target: x86_64-unknown-windows-msvc

libdispatch and Foundation: swift-DEVELOPMENT-SNAPSHOT-2020-08-11-a,

Additional Detail from JIRA
Votes 0
Component/s Foundation, libdispatch
Labels Bug, Dispatch, Foundation, Windows
Assignee None
Priority Medium

md5: 49101804f13730f8eb31fad5750c4923

Issue Description:

Foundation uses CURL for processing network requests, and DispatchSource for listening to socket read/write availability events.

When network request from Foundation times out, DispatchSource is invalidated implicitly (by releasing and deallocating it).

libdispatch then performs cleanup for dispatch source in background thread. The cleanup includes disarming all pending events on a socket. But socket is already invalidated at that moment, and operation fails (WSAEventSelect returns error 10038 - WSAENOTSOCK).

Such error is considered fatal, and libdispatch intentionally crashes.

Sample code

import Foundation
import FoundationNetworking
import XCTest

class TestURLDataTask: XCTestCase {
    func test_timeout() {
        let config = URLSessionConfiguration.default
        config.timeoutIntervalForRequest = 8 // Value doesn't matter, 8 is just to wait less
        let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil)
        let request = URLRequest(url: URL(string: "http://192.168.7.153")!) // use any host that would time out

        let callbackCalled = expectation(description: "Task callback called")
        session.dataTask(with: request, completionHandler: { data, response, error in
            callbackCalled.fulfill()
        }).resume()
        waitForExpectations(timeout: 10)
        Thread.sleep(forTimeInterval: 5)  // as Dispatch crashes asynchronously, give it some time
    }

    static var allTests: [(String, (TestURLDataTask) -> () throws -> Void)] {
        return [("test_timeout", test_timeout),]
    }
}

XCTMain([testCase(TestURLDataTask.allTests)])

Crash location

The crash is in event_windows.c:222

Other dispatch sources

Other dispatch_source implementations (epoll, kevent) also has failable API calls in cleanup code, but intentionally ignore any errors.

It seems to me that socket cleanup have to be more indifferent to WSAEventSelect errors. Or we could ignore WSAENOTSOCK error at least.

Another solution would be somehow do dispatch source cleanup synchronously before socket invalidation in Foundation. Looks like this location in MultiHandle.swift would be a good place to do that. But I don't see any possibility to do it. Available Dispatch API would just mark Source as pending for asynchronous cleanup.

@theblixguy
Copy link

cc @compnerd

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from swiftlang/swift May 5, 2022
@gregcotten
Copy link

@compnerd Swift 5.8 release still does this

@compnerd
Copy link
Member

compnerd commented May 3, 2023

@gregcotten 5.8 is ~1y old, there is a pretty long delay in releases for stability. Any fixes for this would be post 5.9. I think that @lxbndr had dug further and made some progress but I don't think that there was a patch for it yet.

@gregcotten
Copy link

@compnerd, got it! Network request that results in a timeout still causes a crash on April 20, 2023 trunk dev build - compnerd.org Swift version 5.9-dev (LLVM 107de6a91bc1dbd, Swift 022311e438e7703)

@gregcotten
Copy link

Relevant: #772

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

No branches or pull requests

4 participants