Skip to content

fix: handle background transport failures during shutdown - #314

Open
ptgamr wants to merge 1 commit into
connectbot:mainfrom
ptgamr:fix/handle-background-shutdown-failures
Open

ptgamr wants to merge 1 commit into
connectbot:mainfrom
ptgamr:fix/handle-background-shutdown-failures

Conversation

@ptgamr

@ptgamr ptgamr commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Problem

Closing a connection while a channel delivery coroutine is awaiting a window-update write can complete that write with a TransportException rather than CancellationException. ForwardingChannel.incomingDeliveryJob and the session delivery workers let that exception escape their launch coroutine. On Android, the uncaught exception can terminate the app during otherwise normal disconnect/reconnect.

A nested transport can also throw from close() when it tries to send CHANNEL_CLOSE through an already disconnected gateway. That exception currently escapes closeTransport() and interrupts the remaining teardown.

This was reproduced with cbssh 0.5.0 by repeatedly connecting through an SSH gateway, executing printf connected, and closing the route. The command succeeds, but asynchronous teardown can fail afterward. A JVM reproducer that captures uncaught background exceptions also fails without the patch. The affected code is unchanged on current main.

Changes

  • Route forwarding/session delivery failures through the existing connection-failure path and close their output streams with the original cause.
  • Forward buffered stdout failures to readers instead of throwing from its adapter coroutine.
  • Preserve coroutine cancellation semantics in delivery workers.
  • Complete connection cleanup even if the underlying transport throws while closing.
  • Add deterministic regressions for failed forwarding/session window updates (including buffered stdout) and a throwing nested transport close; verify repeated close remains idempotent and later writes fail.

The public API is unchanged.

Validation

  • ./gradlew build passed on a branch based on upstream d2e2936: 1,280 library tests passed, 6 skipped; 11 protocol tests passed. Formatting and Metalava compatibility checks passed. Gradle reused the previously passing test results for identical inputs.
  • Downstream validation of the same patch against 0.5.0: the combined nine-test Android emulator/OpenSSH Docker suite passed twice. It covers one, two and three gateways, password/key/mixed authentication, commands, SFTP, local forwarding, failure cases, 30 close cycles and 30 suspend/reconnect cycles per run.
  • A three-gateway Android UI connection, close and reopen completed without an app crash.

Local build note: supplied the Mockito public key already trusted by upstream verification metadata because its keyserver lookup was unavailable. Signature verification stayed enabled; no keyring or verification metadata changes are included in this PR.

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.

1 participant