Skip to content

Fix socket lifecycle management #5303

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.

Sign up for GitHub

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

Open
clintonpi wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix socket lifecycle management #5303

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
88ce3a6
Fix socket lifecycle management
clintonpi Dec 9, 2025
838b0bf
Correctly schedule socket closure
clintonpi Dec 10, 2025
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
18 changes: 7 additions & 11 deletions Sources/FoundationNetworking/URLSession/libcurl/MultiHandle.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,22 +195,19 @@ private extension URLSession._MultiHandle {
}

/// Removes socket reference from the shared store. If there is work item scheduled,
/// executes it on the current thread.
/// executes it on the current thread. Then, schedules the socket for closure.
func endOperation(for socketReference: _SocketReference) {
precondition(socketReferences.removeValue(forKey: socketReference.socket) != nil, "No operation associated with the socket")
if let workItem = socketReference.workItem, !workItem.isCancelled {
// CURL never asks for socket close without unregistering first, and
// we should cancel pending work when unregister action is requested.
precondition(!socketReference.shouldClose, "Socket close was scheduled, but there is some pending work left")
workItem.perform()
}
}

/// Marks this reference to close socket on deinit. This allows us
/// to extend socket lifecycle by keeping the reference alive.
func scheduleClose(for socket: CFURLSession_socket_t) {
let reference = socketReferences[socket] ?? _SocketReference(socket: socket)
reference.shouldClose = true
/// Marks this reference to close socket on deinit. This allows us
/// to extend socket lifecycle by keeping the reference alive.
socketReference.shouldClose = true
}
Copy link
Contributor

@jrflat jrflat Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we set .shouldClose = true outside of the if let workItem = ... block? It looks like tearDown(handle:socket:queue) will call handle.endOperation(for: socketReference) where socketReference.workItem is nil.

}

/// Schedules work to be performed when an operation ends for the socket,
Expand Down Expand Up @@ -253,10 +250,9 @@ internal extension URLSession._MultiHandle {
// the connection cache is managed by CURL multi_handle, and sockets can actually
// outlive easy_handle (even after curl_easy_cleanup call). That's why
// socket management lives in _MultiHandle.
try! CFURLSession_easy_setopt_ptr(handle.rawHandle, CFURLSessionOptionCLOSESOCKETDATA, UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque())).asError()
try! CFURLSession_easy_setopt_scl(handle.rawHandle, CFURLSessionOptionCLOSESOCKETFUNCTION) { (clientp: UnsafeMutableRawPointer?, item: CFURLSession_socket_t) in
guard let handle = URLSession._MultiHandle.from(callbackUserData: clientp) else { fatalError() }
handle.scheduleClose(for: item)
// Override close(3)/closesocket(3) to keep socket open.
// CFURLSession_socket_t will be scheduled for closure when endOperation(: _SocketReference) is called.
return 0
}.asError()

Expand Down