public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
* [newlib-cygwin] Cygwin: serial: revamp overlapped IO in read and select
@ 2020-03-26 11:26 Corinna Vinschen
0 siblings, 0 replies; only message in thread
From: Corinna Vinschen @ 2020-03-26 11:26 UTC (permalink / raw)
To: cygwin-cvs
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=29295995909f0a56081a77e33fd95f34c4af9335
commit 29295995909f0a56081a77e33fd95f34c4af9335
Author: Corinna Vinschen <corinna@vinschen.de>
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 <corinna@vinschen.de>
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)
{}
};
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2020-03-26 11:26 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26 11:26 [newlib-cygwin] Cygwin: serial: revamp overlapped IO in read and select Corinna Vinschen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).