https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=29295995909f0a56081a77e33fd95f34c4af9335
commit 29295995909f0a56081a77e33fd95f34c4af9335 Author: Corinna Vinschen <[email protected]> Date: Wed Mar 18 21:14:06 2020 +0100 Cygwin: serial: revamp overlapped IO in read and select Get rid of WaitCommEvent and using overlapped_armed to share the same overlapped operation between read and select. Rather, make sure to cancel the overlapped IO before leaving any of these functions. Signed-off-by: Corinna Vinschen <[email protected]> Diff: --- winsup/cygwin/fhandler.h | 1 - winsup/cygwin/fhandler_serial.cc | 111 +++++++++++---------------- winsup/cygwin/select.cc | 161 +++++++++------------------------------ winsup/cygwin/select.h | 9 +-- 4 files changed, 84 insertions(+), 198 deletions(-) diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h index 64a052ce1..923313df4 100644 --- a/winsup/cygwin/fhandler.h +++ b/winsup/cygwin/fhandler.h @@ -1687,7 +1687,6 @@ class fhandler_serial: public fhandler_base int dtr; /* for Windows 9x purposes only */ public: - int overlapped_armed; OVERLAPPED io_status; /* Constructor */ diff --git a/winsup/cygwin/fhandler_serial.cc b/winsup/cygwin/fhandler_serial.cc index 7492470f5..72cb1678d 100644 --- a/winsup/cygwin/fhandler_serial.cc +++ b/winsup/cygwin/fhandler_serial.cc @@ -35,15 +35,14 @@ fhandler_serial::overlapped_setup () memset (&io_status, 0, sizeof (io_status)); io_status.hEvent = CreateEvent (&sec_none_nih, TRUE, FALSE, NULL); ProtectHandle (io_status.hEvent); - overlapped_armed = 0; } void __reg3 fhandler_serial::raw_read (void *ptr, size_t& ulen) { - DWORD io_err, event; + DWORD io_err; COMSTAT st; - DWORD bytes_to_read, read_bytes, undefined; + DWORD bytes_to_read, read_bytes; ssize_t tot = 0; if (ulen > SSIZE_MAX) @@ -57,11 +56,6 @@ fhandler_serial::raw_read (void *ptr, size_t& ulen) debug_printf ("ulen %ld, vmin_ %u, vtime_ %u, hEvent %p", ulen, vmin_, vtime_, io_status.hEvent); - if (!overlapped_armed) - { - SetCommMask (get_handle (), EV_RXCHAR); - ResetEvent (io_status.hEvent); - } do { @@ -93,58 +87,8 @@ fhandler_serial::raw_read (void *ptr, size_t& ulen) and don't wait. */ if (st.cbInQue && st.cbInQue >= minchars) bytes_to_read = MIN (st.cbInQue, bytes_to_read); - /* if vtime_ is set, use kernel timeouts, otherwise wait here. */ - else if (vtime_ == 0 && !overlapped_armed) - { - if (WaitCommEvent (get_handle (), &event, &io_status)) - { - debug_printf ("WaitCommEvent succeeded: event %x", event); - if (!event) - continue; - } - else if (GetLastError () != ERROR_IO_PENDING) - goto err; - overlapped_armed = 1; - } - } - /* overlapped_armed may be set by select, so we have to make sure - to disarm even in O_NONBLOCK mode. */ - if (overlapped_armed) - { - switch (cygwait (io_status.hEvent, is_nonblocking () ? 0 : INFINITE)) - { - case WAIT_OBJECT_0: - if (!GetOverlappedResult (get_handle (), &io_status, - &undefined, TRUE)) - goto err; - debug_printf ("overlapped event %x", event); - break; - case WAIT_SIGNALED: - CancelIo (get_handle ()); - overlapped_armed = 0; - if (!GetOverlappedResult (get_handle (), &io_status, - &undefined, TRUE)) - goto err; - /* Only if no bytes read, return with EINTR. */ - if (!tot) - { - tot = -1; - set_sig_errno (EINTR); - } - goto out; - case WAIT_CANCELED: - CancelIo (get_handle ()); - overlapped_armed = 0; - GetOverlappedResult (get_handle (), &io_status, &undefined, - TRUE); - pthread::static_cancel_self (); - /*NOTREACHED*/ - default: - goto err; - } } - overlapped_armed = 0; ResetEvent (io_status.hEvent); if (!ReadFile (get_handle (), ptr, bytes_to_read, &read_bytes, &io_status)) @@ -152,10 +96,50 @@ fhandler_serial::raw_read (void *ptr, size_t& ulen) if (GetLastError () != ERROR_IO_PENDING) goto err; if (is_nonblocking ()) - CancelIo (get_handle ()); - if (!GetOverlappedResult (get_handle (), &io_status, &read_bytes, - TRUE)) - goto err; + { + CancelIo (get_handle ()); + if (!GetOverlappedResult (get_handle (), &io_status, &read_bytes, + TRUE)) + goto err; + } + else + { + switch (cygwait (io_status.hEvent)) + { + default: /* Handle an error case from cygwait basically like + a cancel condition and see if we got "something" */ + CancelIo (get_handle ()); + /*FALLTHRU*/ + case WAIT_OBJECT_0: + if (!GetOverlappedResult (get_handle (), &io_status, + &read_bytes, TRUE)) + goto err; + debug_printf ("got %u bytes from ReadFile", read_bytes); + break; + case WAIT_SIGNALED: + CancelIo (get_handle ()); + if (!GetOverlappedResult (get_handle (), &io_status, + &read_bytes, TRUE)) + goto err; + /* Only if no bytes read, return with EINTR. */ + if (!tot && !read_bytes) + { + tot = -1; + set_sig_errno (EINTR); + debug_printf ("signal received, set EINTR"); + } + else + debug_printf ("signal received but ignored"); + goto out; + case WAIT_CANCELED: + CancelIo (get_handle ()); + GetOverlappedResult (get_handle (), &io_status, &read_bytes, + TRUE); + debug_printf ("thread canceled"); + pthread::static_cancel_self (); + /*NOTREACHED*/ + } + } } tot += read_bytes; ptr = (void *) ((caddr_t) ptr + read_bytes); @@ -174,9 +158,6 @@ fhandler_serial::raw_read (void *ptr, size_t& ulen) tot = -1; __seterrno (); } - CancelIo (get_handle ()); - overlapped_armed = 0; - GetOverlappedResult (get_handle (), &io_status, &undefined, TRUE); break; } } diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc index 6ffc97b37..93820ae77 100644 --- a/winsup/cygwin/select.cc +++ b/winsup/cygwin/select.cc @@ -1434,22 +1434,15 @@ fhandler_dev_null::select_except (select_stuff *ss) return s; } -static int start_thread_serial (select_record *me, select_stuff *stuff); - static int peek_serial (select_record *s, bool) { + HANDLE h; COMSTAT st; - DWORD event, io_err; + DWORD io_err; fhandler_serial *fh = (fhandler_serial *) s->fh; - if (fh->get_readahead_valid ()) - return s->read_ready = true; - - select_printf ("fh->overlapped_armed %d", fh->overlapped_armed); - - HANDLE h; set_handle_or_return_if_not_open (h, s); if ((s->read_selected && s->read_ready) @@ -1459,55 +1452,16 @@ peek_serial (select_record *s, bool) return true; } - /* This is apparently necessary for the com0com driver. - See: http://cygwin.com/ml/cygwin/2009-01/msg00667.html */ - SetCommMask (h, 0); - - SetCommMask (h, EV_RXCHAR); - - if (!fh->overlapped_armed) - { - COMSTAT st; - - ResetEvent (fh->io_status.hEvent); - - if (!ClearCommError (h, &io_err, &st)) - { - debug_printf ("ClearCommError %E"); - goto err; - } - if (st.cbInQue) - return s->read_ready = true; - if (WaitCommEvent (h, &event, &fh->io_status)) - return s->read_ready = true; - if (GetLastError () != ERROR_IO_PENDING) - { - debug_printf ("WaitCommEvent %E"); - goto err; - } - fh->overlapped_armed = 1; - } + if (fh->get_readahead_valid ()) + return s->read_ready = true; - switch (WaitForSingleObject (fh->io_status.hEvent, 10L)) + if (!ClearCommError (h, &io_err, &st)) { - case WAIT_OBJECT_0: - if (!ClearCommError (h, &io_err, &st)) - { - debug_printf ("ClearCommError %E"); - goto err; - } - if (st.cbInQue) - { - select_printf ("got something"); - return s->read_ready = true; - } - break; - case WAIT_TIMEOUT: - break; - default: - debug_printf ("WaitForMultipleObjects %E"); + debug_printf ("ClearCommError %E"); goto err; } + if (st.cbInQue) + return s->read_ready = true; return 0; @@ -1515,84 +1469,49 @@ err: if (GetLastError () == ERROR_OPERATION_ABORTED) { select_printf ("operation aborted"); - return 0; + return false; } s->set_select_errno (); return -1; } -static DWORD WINAPI -thread_serial (void *arg) +static void +serial_read_cleanup (select_record *s, select_stuff *stuff) { - select_serial_info *si = (select_serial_info *) arg; - bool looping = true; + if (s->h) + { + fhandler_serial *fh = (fhandler_serial *) s->fh; + HANDLE h = fh->get_handle_cyg (); + DWORD undefined; - while (looping) - for (select_record *s = si->start; (s = s->next); ) - if (s->startup != start_thread_serial) - continue; - else + if (h) { - if (peek_serial (s, true)) - looping = false; - if (si->stop_thread) - { - select_printf ("stopping"); - looping = false; - break; - } + CancelIo (h); + GetOverlappedResult (h, &fh->io_status, &undefined, TRUE); } - - select_printf ("exiting"); - return 0; -} - -static int -start_thread_serial (select_record *me, select_stuff *stuff) -{ - if (stuff->device_specific_serial) - me->h = *((select_serial_info *) stuff->device_specific_serial)->thread; - else - { - select_serial_info *si = new select_serial_info; - si->start = &stuff->start; - si->stop_thread = false; - si->thread = new cygthread (thread_serial, si, "sersel"); - me->h = *si->thread; - stuff->device_specific_serial = si; - } - return 1; -} - -static void -serial_cleanup (select_record *, select_stuff *stuff) -{ - select_serial_info *si = (select_serial_info *) stuff->device_specific_serial; - if (!si) - return; - if (si->thread) - { - si->stop_thread = true; - si->thread->detach (); } - delete si; - stuff->device_specific_serial = NULL; } select_record * fhandler_serial::select_read (select_stuff *ss) { + DWORD event; select_record *s = ss->start.next; - if (!s->startup) - { - s->startup = start_thread_serial; - s->verify = verify_ok; - s->cleanup = serial_cleanup; - } + + s->startup = no_startup; + s->verify = verify_ok; + s->cleanup = serial_read_cleanup; s->peek = peek_serial; s->read_selected = true; s->read_ready = false; + /* This is apparently necessary for the com0com driver. + See: http://cygwin.com/ml/cygwin/2009-01/msg00667.html */ + SetCommMask (get_handle_cyg (), 0); + SetCommMask (get_handle_cyg (), EV_RXCHAR); + if (!WaitCommEvent (get_handle_cyg (), &event, &io_status) + && GetLastError () == ERROR_IO_PENDING) + s->h = io_status.hEvent; return s; } @@ -1600,13 +1519,10 @@ select_record * fhandler_serial::select_write (select_stuff *ss) { select_record *s = ss->start.next; - if (!s->startup) - { - s->startup = no_startup; - s->verify = verify_ok; - } + + s->startup = no_startup; + s->verify = verify_ok; s->peek = peek_serial; - s->h = get_handle (); s->write_selected = true; s->write_ready = true; return s; @@ -1616,12 +1532,9 @@ select_record * fhandler_serial::select_except (select_stuff *ss) { select_record *s = ss->start.next; - if (!s->startup) - { - s->startup = no_startup; - s->verify = verify_ok; - } - s->h = NULL; + + s->startup = no_startup; + s->verify = verify_ok; s->peek = peek_serial; s->except_selected = false; // Can't do this s->except_ready = false; diff --git a/winsup/cygwin/select.h b/winsup/cygwin/select.h index 98fde3a89..886810a0b 100644 --- a/winsup/cygwin/select.h +++ b/winsup/cygwin/select.h @@ -67,11 +67,6 @@ struct select_socket_info: public select_info select_socket_info (): select_info (), num_w4 (0), ser_num (0), w4 (NULL) {} }; -struct select_serial_info: public select_info -{ - select_serial_info (): select_info () {} -}; - class select_stuff { public: @@ -92,7 +87,6 @@ public: select_pipe_info *device_specific_ptys; select_fifo_info *device_specific_fifo; select_socket_info *device_specific_socket; - select_serial_info *device_specific_serial; bool test_and_set (int, fd_set *, fd_set *, fd_set *); int poll (fd_set *, fd_set *, fd_set *); @@ -105,8 +99,7 @@ public: device_specific_pipe (NULL), device_specific_ptys (NULL), device_specific_fifo (NULL), - device_specific_socket (NULL), - device_specific_serial (NULL) + device_specific_socket (NULL) {} };
