apple / swift-nio-ssh

SwiftNIO SSH is a programmatic implementation of SSH using SwiftNIO
Apache License 2.0
398 stars 49 forks source link

Crash in `SSHChildChannel` #143

Closed gaetanzanella closed 1 year ago

gaetanzanella commented 1 year ago

SwiftNIO SSH commit hash: c56ababb0e22e0cdf5ec8b31698e07f943ea92e7

Context

While writing Xcode tests in one of my project which uses swift-nio-ssh, I encountered a crash. I reproduced it using the NIOSSHClient target.

Steps to reproduce

  1. Launch the ssh server: docker-compose -f ./docker/ssh-docker-compose.yaml up -d
  2. Run the modified NIOSSHClient target.
  3. Observe the crash in Xcode
    * thread #2, name = 'NIO-ELT-0-#0', stop reason = Fatal error: Sent channel window adjust on channel in invalid state
    frame #0: 0x0000000193fe4ae8 libswiftCore.dylib`_swift_runtime_on_report
    frame #1: 0x00000001940825d4 libswiftCore.dylib`_swift_stdlib_reportFatalErrorInFile + 208
    frame #2: 0x0000000193c614b4 libswiftCore.dylib`closure #1 (Swift.UnsafeBufferPointer<Swift.UInt8>) -> () in closure #1 (Swift.UnsafeBufferPointer<Swift.UInt8>) -> () in Swift._assertionFailure(_: Swift.StaticString, _: Swift.String, file: Swift.StaticString, line: Swift.UInt, flags: Swift.UInt32) -> Swift.Never + 196
    frame #3: 0x0000000193c60210 libswiftCore.dylib`Swift._assertionFailure(_: Swift.StaticString, _: Swift.String, file: Swift.StaticString, line: Swift.UInt, flags: Swift.UInt32) -> Swift.Never + 308
    * frame #4: 0x00000001002cae7c NIOSSHClient`ChildChannelStateMachine.sendChannelWindowAdjust(message=(recipientChannel = 0, bytesToAdd = 8811376), self=NIOSSH.ChildChannelStateMachine @ 0x0000000101e040a8) at ChildChannelStateMachine.swift:432:13
    ...

    Configuration

    $ swift --version swift-driver version: 1.75.2 Apple Swift version 5.8 (swiftlang-5.8.0.124.2 clang-1403.0.22.11.100) Target: arm64-apple-macosx13.0

Operating system: macOS Ventura 13.2

Xcode version: 14.2

$ uname -a Darwin MBP-de-Gaetan-Zanella 22.3.0 Darwin Kernel Version 22.3.0: Thu Jan 5 20:48:54 PST 2023; root:xnu-8792.81.2~2/RELEASE_ARM64_T6000 arm64

Lukasa commented 1 year ago

Hmm, I'm not observing this crash locally. Can you attach a full backtrace, or even better a crash report?

gaetanzanella commented 1 year ago

NIOSSHClient-2023-04-17-164811.ips.zip

You can try to reduce the time before closing:

DispatchQueue.global(qos: .default).asyncAfter(deadline: .now() + 0.5) {
    print("❌")
    _ = childChannel.close(promise: nil)
}

I propose a new version (I think the print was creating some delay).

Lukasa commented 1 year ago

Ok great, this crash report is sufficient. I think the issue here is that we don't change that self.state.sentClose is false before we send the window update. That would likely fix the issue here. It shouldn't be too hard to construct a unit test that triggers this.

gaetanzanella commented 1 year ago

I tried a fix. I am not super sure about it:

Let me know. Thanks for your time!