-
Notifications
You must be signed in to change notification settings - Fork 361
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
DefaultScpClient.upload(InputStream, ...) will always hit the exit status timeout #427
Comments
I should update that this is observed against an OpenSSH server with a default client side SSH configuration. From the large logging you can observe the exit timeout delay here:
|
This is also easily reproducible using the including unit test Moving the I am still not sure why only You can simulate the issue by introducing a 200ms delay between the IO streams and the channel closing. Hopefully this information is helpful. ClientSession session = getClientSession();
ChannelExec channel = openCommandChannel(session, cmd);
// Wrapped with an additional try/finally.
try {
try (InputStream invOut = channel.getInvertedOut();
OutputStream invIn = channel.getInvertedIn()) {
// NOTE: we use a mock file system since we expect no invocations for it
ScpHelper helper = new ScpHelper(session, invOut, invIn, new MockFileSystem(remote), opener, listener);
Path mockPath = new MockPath(remote);
helper.sendStream(new DefaultScpStreamResolver(name, mockPath, perms, time, size, local, cmd),
options.contains(Option.PreserveAttributes), ScpHelper.DEFAULT_SEND_BUFFER_SIZE);
// handleCommandExitStatus(cmd, channel);
}
Thread.sleep(200);
// handleCommandExitStatus(cmd, channel);
} catch (InterruptedException e) {
throw new RuntimeException(e);
} finally {
channel.close(false);
}
|
With some further testing this appears to happen if we trigger the sending of an EOF too quickly after sending a stream. Placing a 100ms delay between these actions alleviates the issue and further demonstrates the race condition. try {
try (InputStream invOut = channel.getInvertedOut();
OutputStream invIn = channel.getInvertedIn()) {
// NOTE: we use a mock file system since we expect no invocations for it
ScpHelper helper = new ScpHelper(session, invOut, invIn, new MockFileSystem(remote), opener, listener);
Path mockPath = new MockPath(remote);
helper.sendStream(new DefaultScpStreamResolver(name, mockPath, perms, time, size, local, cmd),
options.contains(Option.PreserveAttributes), ScpHelper.DEFAULT_SEND_BUFFER_SIZE);
Thread.sleep(100);
}
handleCommandExitStatus(cmd, channel);
} catch (InterruptedException e) {
throw new RuntimeException(e);
} finally {
channel.close(false);
} It appears that the server is responding with an unexpected additional ACK after sending the payload stream and this message is racing with the channel closure.
|
It appears that this is caused by an initial ACK not being processed when the channel is opened, then the following sequence of ACKs are not processed in the correct order, which leaves a trailing ACK at the end to race with channel closure.
I will submit a PR, though I am not sure of the protocol here so please ensure this fix would not be server dependent. |
GH-427: Read initial ACK on channel open prior to direct stream upload & close streams prior to exit code handling
Version
2.11.0
Bug description
When attempting to upload/scp a byte stream using the below method, the main thread will always block until the exit status timeout is reached.
DefaultScpClient::upload(InputStream local, String remote, long size,Collection<PosixFilePermission> perms, ScpTimestampCommandDetails time)
It appears this is the case because the handleCommandExitStatus(cmd, channel) is called before the In/Out channels are auto-closed by the try-with-resources block, and thus the EOF is not sent to the remote prior to handling the exit status. The current implementation always times out first and then sends an EOF while closing the channels.
Using the API to send based on file paths will invoke DefaultScpClient::runUpload() which first closes the channel resources and then waits for the exit status. This API works well without the exit status blocking the thread.
I have overridden the default client to ignore the exit status result, and there is a dedicated property to adjust this timeout. Though, this is not really a solution to this issue.
This issue is persistent on remote SSH servers running with CentOS and MacOS. This example is being run on MacOS 13.3.1 with a remote SSH server that has no explicit SSH configuration. Testing on sshd-scp 2.10.0 and 2.11.0.
Overriding the client to do the following seems to resolve the issue:
Actual behavior
The upload method blocked the main thread after byte transfer for 5 seconds.
Expected behavior
The upload SCP routine should receive an exit status immediately after byte array upload.
Relevant log output
Other information
No response
The text was updated successfully, but these errors were encountered: