diff options
| -rw-r--r-- | winsup/cygwin/ChangeLog | 6 | ||||
| -rw-r--r-- | winsup/cygwin/DevNotes | 32 | ||||
| -rw-r--r-- | winsup/cygwin/fhandler_tty.cc | 17 | ||||
| -rw-r--r-- | winsup/cygwin/release/1.7.16 | 6 |
4 files changed, 53 insertions, 8 deletions
diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 1511306e5..f27762a39 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,9 @@ +2012-05-15 Christopher Faylor <me.cygwin2012@cgf.cx> + + * DevNotes: Add entry cgf-000008. + * fhandler_tty.cc (bytes_available): Simplify by returning the number + of bytes available in the message unless that is zero. + 2012-05-14 Christopher Faylor <me.cygwin2012@cgf.cx> * child_info.h (CURR_CHILD_INFO_MAGIC): Update. diff --git a/winsup/cygwin/DevNotes b/winsup/cygwin/DevNotes index e1d2b3f61..da9cea13e 100644 --- a/winsup/cygwin/DevNotes +++ b/winsup/cygwin/DevNotes @@ -1,3 +1,35 @@ +2012-05-14 cgf-000008 + +<1.7.16> +- Fix hang when zero bytes are written to a pty using + Windows WriteFile or equivalent. Fixes: + http://cygwin.com/ml/cygwin/2012-05/msg00323.html +</1.7.16> + +cgf-000002, as usual, fixed one thing while breaking another. See +Larry's predicament in: http://goo.gl/oGEr2 . + +The problem is that zero byte writes to the pty pipe caused the dread +end-of-the-world-as-we-know-it problem reported on the mailing list +where ReadFile reads zero bytes even though there is still more to read +on the pipe. This is because that change caused a 'record' to be read +and a record can be zero bytes. + +I was never really keen about using a throwaway buffer just to get a +count of the number of characters available to be read in the pty pipe. +On closer reading of the documentation for PeekNamedPipe it seemed like +the sixth argument to PeekNamedPipe should return what I needed without +using a buffer. And, amazingly, it did, except that the problem still +remained - a zero byte message still screwed things up. + +So, we now detect the case where there is zero bytes available as a +message but there are bytes available in the pipe. In that scenario, +return the bytes available in the pipe rather than the message length of +zero. This could conceivably cause problems with pty pipe handling in +this scenario but since the only way this scenario could possibly happen +is when someone is writing zero bytes using WriteFile to a pty pipe, I'm +ok with that. + 2012-05-14 cgf-000007 <1.7.16> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc index 1428a064e..38a0571cb 100644 --- a/winsup/cygwin/fhandler_tty.cc +++ b/winsup/cygwin/fhandler_tty.cc @@ -53,17 +53,20 @@ fhandler_pty_slave::get_unit () bool bytes_available (DWORD& n, HANDLE h) { - char buf[INP_BUFFER_SIZE]; - /* Apparently need to pass in a dummy buffer to read a real "record" from - the pipe. So buf is used and then discarded just so we can see how many - bytes will be read by the next ReadFile(). */ - bool succeeded = PeekNamedPipe (h, buf, sizeof (buf), &n, NULL, NULL); - if (!succeeded) + DWORD navail, nleft; + navail = nleft = 0; + bool succeeded = PeekNamedPipe (h, NULL, 0, NULL, &navail, &nleft); + if (succeeded) + /* nleft should always be the right choice unless something has written 0 + bytes to the pipe. In that pathological case we return the actual number + of bytes available in the pipe. See cgf-000008 for more details. */ + n = nleft ?: navail; + else { termios_printf ("PeekNamedPipe(%p) failed, %E", h); n = 0; } - debug_only_printf ("%u bytes available", n); + debug_only_printf ("n %u, nleft %u, navail %u"); return succeeded; } diff --git a/winsup/cygwin/release/1.7.16 b/winsup/cygwin/release/1.7.16 index 5656671f7..31f2da485 100644 --- a/winsup/cygwin/release/1.7.16 +++ b/winsup/cygwin/release/1.7.16 @@ -7,10 +7,14 @@ Bug fixes: ---------- - Fix pipe creation problem which manifested as a problem creating a -fifo. Fixes: http://cygwin.com/ml/cygwin/2012-05/msg00253.html + fifo. Fixes: http://cygwin.com/ml/cygwin/2012-05/msg00253.html - Fix hang when calling pthread_testcancel in a canceled thread. Fixes some of: http://cygwin.com/ml/cygwin/2012-05/msg00186.html - Fix invocation of strace from a cygwin process. Fixes: http://cygwin.com/ml/cygwin/2012-05/msg00292.html + +- Fix hang when zero bytes are written to a pty using + Windows WriteFile or equivalent. Fixes: + http://cygwin.com/ml/cygwin/2012-05/msg00323.html |