Race condition causes ScpTest.testScpNativeOnMultipleFiles() to fail (flaky test)

[tomaswolf]

The ultimate problem is a general one concerning ChannelExecs:

If

• there is a communication protocol implemented between the client and the remote command via the stdin/stdout streams,
• and if the protocol is such that the remote command can return an error code in this application protocol,
• and the client uses the default inverted output stream to read that error code,
• and the client then decides to close the channel (even gracefully),

then it is possible that the AbstractClientChannel's out stream is closed between lines 436 and 437, and then the flush()call on the ChannelPipedOutputStream may fail, causing the whole session to go down.

This can happen in the ScpTestat line 343, which tests an scp upload command that should fail. The scp server sends back an ERROR ack, which the client reads (on an I/O thread) and forwards in AbstractClientChannel. The client reads this though the ChannelPipedInputStream (on the main thread) and throws an exception, which then causes the channel to be closed at the end of DefaultScpClient.runUpload() (still on the main thread). Only then does the I/O thread call flush(), which then fails with an exception.

So in short we have

1. I/O thread: receive data
2. I/O thread: write data to ChannelPipedOutputStream
3. Main thread: read data from ChannelPipedInputStream
4. Main thread: close channel
5. I/O thread: call ChannelPipedOutputStream.flush() and fail.

This is all the more infuriating since ChannelPipedOutputStream.flush() is essentially a no-op. Moreover: an exception when a channel has received data and tries to forward it should not take down the whole session but only this channel.