Skip to content

URLSessionWebSocketTask.receive() not finishing if server closes connection without a close packet #4673

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

Merged
merged 1 commit into from
Jan 25, 2023

Conversation

pseligman
Copy link
Contributor

Two fixes required here:

  • _WebSocketURLProtocol needs to detect a closing URL connection, and set its state to indicate this is a normal close of the connection
  • URLSessionWebSocketTask needs to kick doPendingWork() if the connection is closed out due to an error, in order to respond to any pending Tasks parked in receive()

@pseligman
Copy link
Contributor Author

@jrflat could you please review?

@pseligman
Copy link
Contributor Author

@swift-ci please test

@@ -7,8 +7,13 @@
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//

import Foundation
import XCTest
#if NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was necessary for TestDecimal to build on macOS against the correct Foundation library. See also e9e183c#r1011034654 . I was mistaken in thinking this change was unnecessary.

@pseligman
Copy link
Contributor Author

The macOS build failure appears unrelated to this change?

    dyld: Library not loaded: @rpath/lib_SwiftSyntaxMacros.dylib
      Referenced from: /Users/ec2-user/jenkins/workspace/swift-corelibs-foundation-PR-macOS/branch-main/build/Ninja-ReleaseAssert/toolchain-macosx-x86_64/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swiftc
      Reason: image not found

Copy link
Contributor

@jrflat jrflat left a comment

Choose a reason for hiding this comment

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

Looks good!

@jrflat
Copy link
Contributor

jrflat commented Dec 14, 2022

@swift-ci please test

@pseligman
Copy link
Contributor Author

Same macOS error :-/

    dyld: Library not loaded: @rpath/lib_SwiftSyntaxMacros.dylib
      Referenced from: /Users/ec2-user/jenkins/workspace/swift-corelibs-foundation-PR-macOS/branch-main/build/Ninja-ReleaseAssert/toolchain-macosx-x86_64/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swiftc
      Reason: image not found

@jrflat
Copy link
Contributor

jrflat commented Jan 6, 2023

@swift-ci please test macOS platform

@pseligman
Copy link
Contributor Author

error: --install-xctest is not supported on this platform

Seems the macOS platform build is still having issues unrelated to this change?

@jrflat
Copy link
Contributor

jrflat commented Jan 10, 2023

@swift-ci please test macOS platform

@pseligman
Copy link
Contributor Author

error: --install-xctest is not supported on this platform same issue as last week

@jrflat
Copy link
Contributor

jrflat commented Jan 24, 2023

@swift-ci please test macOS platform

@jrflat
Copy link
Contributor

jrflat commented Jan 25, 2023

Tests look good!

@jrflat jrflat merged commit a1c25ff into swiftlang:main Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants