Fix flushing channel data when pty was allocated #85
Conversation
The first change is flushing data before closing channel->writefd. When pty was used, readfd and writefd are the same. Reading then flushing data will be impossible if readfd was closed first. The second change is to actually flush data with new wrapper function flush_msg_channel_data(). It seems with OpenWrt kernel 4.19.79 at least, a single read() can only fetch 4095 bytes of data. It takes multiple reads to drain the buffer. The last change is in send_msg_channel_data(). The channel fd SHUT_RD logic was removed. Like said above, read less than requested does not imply EOF Ref: https://bugs.openwrt.org/index.php?do=details&task_id=1814
common-channel.c
Outdated
| || (channel->type->check_close && close_allowed)) { | ||
| close_chan_fd(channel, channel->writefd, SHUT_WR); | ||
| } | ||
|
|
||
| /* Special handling for flushing read data after an exit. We | ||
| read regardless of whether the select FD was set, | ||
| and if there isn't data available, the channel will get closed. */ | ||
| if (channel->flushing) { | ||
| TRACE(("might send data, flushing")) | ||
| if (channel->readfd >= 0 && channel->transwindow > 0) { |
There was a problem hiding this comment.
Because it's checking for available channel->transwindow, a larger file might still get truncated?
There was a problem hiding this comment.
It depends on how much data the pty can buffer and how small the transwindow can be. I tested with 800000 bytes file and it worked. But the correct approach should be to drain the buffer by both waiting for readability of readfd and availability of transwinfow.
Feel free to close this when you start working on the new fix :) |
|
Hi @mkj , I added a 2nd change to check for availability of transwindow before doing flush. |
|
Friendly ping. |
The first change is flushing data before closing channel->writefd. When
pty was used, readfd and writefd are the same. Reading then flushing
data will be impossible if readfd was closed first.
The second change is to actually flush data with new wrapper function
flush_msg_channel_data(). It seems with OpenWrt kernel 4.19.79 at
least, a single read() can only fetch 4095 bytes of data. It takes
multiple reads to drain the buffer.
The last change is in send_msg_channel_data(). The channel fd SHUT_RD
logic was removed. Like said above, read less than requested does not
imply EOF
Ref: https://bugs.openwrt.org/index.php?do=details&task_id=1814