swiftlang / swiftlang/swift-corelibs-libdispatch

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

Open
#609 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug Dispatch Foundation libdispatch Windows
Dominant language
C
Stars
2.6k
Forks
496
Avg merge
2d 6h
Merged PRs (30d)
3

Description

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the timeout case from the sample XCTest, then inspect src/event/event_windows.c at line 222 where cleanup reports WSAENOTSOCK. Compare the cleanup behavior with the other dispatch source implementations and review MultiHandle.swift around line 126 for the Foundation interaction. Done means the Windows timeout path no longer causes an asynchronous libdispatch crash.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, swift
Domain
networking, operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.