[PATCH] Re: Cygwin select() issues and improvements
john hood
cgull@glup.org
Sun Mar 13 21:37:00 GMT 2016
On 3/4/16 3:58 AM, Corinna Vinschen wrote:
> John,
>
>
> Ping? I'd be interested to get your patches into Cygwin. select
> really needs some kicking :)
Sorry to be so slow responding. Here's a rebased, squashed,
changelog-ified patch, and I've sent an assignment in to Red Hat.
Aside: Why maintain Git comments in ChangeLog format? That made sense
up into the CVS era, but now version control systems and viewers provide
most of the details that a ChangeLog needed to list, and newlib/Cygwin
has stopped maintaining ChangeLogs.
regards,
--jh
-------------- next part --------------
From 0e407e4206746d8a6d1ee93d680daefa8ac2f00b Mon Sep 17 00:00:00 2001
From: John Hood <cgull@glup.org>
Date: Thu, 28 Jan 2016 17:08:39 -0500
Subject: [PATCH] Improve Cygwin select() implementation.
* fhandler.h: (fhandler_console) Move get_nonascii_key() from
select.c into this class.
* fhandler.cc (fhandler_base::get_readahead): Add debug code.
* fhandler_console.cc (fhandler_console::read): Add debug code.
* select.h: Eliminate redundant select_stuff::select_loop state.
Change prototype for select_stuff::wait() for larger microsecond
timeouts.
* select.cc (pselect): Convert from old cygwin_select().
Implement microsecond timeouts. Add debug code.
(cygwin_select): Rewrite as a wrapper on pselect().
(select): Implement microsecond timeouts. Eliminate redundant
select_stuff::select_loop state. Eliminate redundant code for
zero timeout. Do not return early on early timer return.
(select_stuff::wait): Implement microsecond timeouts with a timer
object. Eliminate redundant select_stuff::select_loop state.
(peek_console): Move get_nonascii_key() into fhandler_console
class. Add debug code.
---
winsup/cygwin/fhandler.cc | 1 +
winsup/cygwin/fhandler.h | 1 +
winsup/cygwin/fhandler_console.cc | 13 +-
winsup/cygwin/select.cc | 254 ++++++++++++++++++++------------------
winsup/cygwin/select.h | 3 +-
winsup/testsuite/configure | 0
6 files changed, 149 insertions(+), 123 deletions(-)
mode change 100644 => 100755 winsup/testsuite/configure
diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
index 33743d4..86f77c3 100644
--- a/winsup/cygwin/fhandler.cc
+++ b/winsup/cygwin/fhandler.cc
@@ -90,6 +90,7 @@ fhandler_base::get_readahead ()
/* FIXME - not thread safe */
if (raixget >= ralen)
raixget = raixput = ralen = 0;
+ debug_printf("available: %d", chret > -1);
return chret;
}
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 134fd71..bad63f2 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1463,6 +1463,7 @@ private:
bool set_unit ();
static bool need_invisible ();
static void free_console ();
+ static const char *get_nonascii_key (INPUT_RECORD& input_rec, char *);
fhandler_console (void *) {}
diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index a52449e..3cf2f41 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -46,8 +46,6 @@ details. */
#define srTop (con.b.srWindow.Top + con.scroll_region.Top)
#define srBottom ((con.scroll_region.Bottom < 0) ? con.b.srWindow.Bottom : con.b.srWindow.Top + con.scroll_region.Bottom)
-const char *get_nonascii_key (INPUT_RECORD&, char *);
-
const unsigned fhandler_console::MAX_WRITE_CHARS = 16384;
fhandler_console::console_state NO_COPY *fhandler_console::shared_console_info;
@@ -310,7 +308,9 @@ fhandler_console::read (void *pv, size_t& buflen)
int ch;
set_input_state ();
+ debug_printf("requested buflen %d", buflen);
int copied_chars = get_readahead_into_buffer (buf, buflen);
+ debug_printf("copied_chars %d", copied_chars);
if (copied_chars)
{
@@ -688,9 +688,11 @@ fhandler_console::read (void *pv, size_t& buflen)
continue;
}
+ debug_printf("toadd = %p, nread = %d", toadd, nread);
if (toadd)
{
- line_edit_status res = line_edit (toadd, nread, ti);
+ ssize_t bytes_read;
+ line_edit_status res = line_edit (toadd, nread, ti, &bytes_read);
if (res == line_edit_signalled)
goto sig_exit;
else if (res == line_edit_input_done)
@@ -698,6 +700,8 @@ fhandler_console::read (void *pv, size_t& buflen)
}
}
+ debug_printf("ralen = %d, bytes = %d", ralen, ralen - raixget);
+
while (buflen)
if ((ch = get_readahead ()) < 0)
break;
@@ -709,6 +713,7 @@ fhandler_console::read (void *pv, size_t& buflen)
#undef buf
buflen = copied_chars;
+ debug_printf("buflen set to %d", buflen);
return;
err:
@@ -2377,7 +2382,7 @@ static const struct {
};
const char *
-get_nonascii_key (INPUT_RECORD& input_rec, char *tmp)
+fhandler_console::get_nonascii_key (INPUT_RECORD& input_rec, char *tmp)
{
#define NORMAL 0
#define SHIFT 1
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index e1d48a3..4bfc484 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -32,7 +32,6 @@ details. */
#include "pinfo.h"
#include "sigproc.h"
#include "cygtls.h"
-#include "cygwait.h"
/*
* All these defines below should be in sys/types.h
@@ -85,41 +84,68 @@ details. */
return -1; \
}
-static int select (int, fd_set *, fd_set *, fd_set *, DWORD);
+static int select (int, fd_set *, fd_set *, fd_set *, LONGLONG);
/* The main select code. */
extern "C" int
-cygwin_select (int maxfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
- struct timeval *to)
+pselect(int maxfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
+ const struct timespec *to, const sigset_t *set)
{
- select_printf ("select(%d, %p, %p, %p, %p)", maxfds, readfds, writefds, exceptfds, to);
+ sigset_t oldset = _my_tls.sigmask;
- pthread_testcancel ();
- int res;
- if (maxfds < 0)
- {
- set_errno (EINVAL);
- res = -1;
- }
- else
+ __try
{
- /* Convert to milliseconds or INFINITE if to == NULL */
- DWORD ms = to ? (to->tv_sec * 1000) + (to->tv_usec / 1000) : INFINITE;
- if (ms == 0 && to->tv_usec)
- ms = 1; /* At least 1 ms granularity */
+ if (set)
+ set_signal_mask (_my_tls.sigmask, *set);
+
+ select_printf ("pselect(%d, %p, %p, %p, %p, %p)", maxfds, readfds, writefds, exceptfds, to, set);
- if (to)
- select_printf ("to->tv_sec %ld, to->tv_usec %ld, ms %d", to->tv_sec, to->tv_usec, ms);
+ pthread_testcancel ();
+ int res;
+ if (maxfds < 0)
+ {
+ set_errno (EINVAL);
+ res = -1;
+ }
else
- select_printf ("to NULL, ms %x", ms);
+ {
+ /* Convert to microseconds or -1 if to == NULL */
+ LONGLONG us = to ? to->tv_sec * 1000000LL + (to->tv_nsec + 999) / 1000 : -1LL;
- res = select (maxfds, readfds ?: allocfd_set (maxfds),
- writefds ?: allocfd_set (maxfds),
- exceptfds ?: allocfd_set (maxfds), ms);
+ if (to)
+ select_printf ("to->tv_sec %ld, to->tv_nsec %ld, us %lld", to->tv_sec, to->tv_nsec, us);
+ else
+ select_printf ("to NULL, us %lld", us);
+
+ res = select (maxfds, readfds ?: allocfd_set (maxfds),
+ writefds ?: allocfd_set (maxfds),
+ exceptfds ?: allocfd_set (maxfds), us);
+ }
+ syscall_printf ("%R = select(%d, %p, %p, %p, %p)", res, maxfds, readfds,
+ writefds, exceptfds, to);
+
+ if (set)
+ set_signal_mask (_my_tls.sigmask, oldset);
+ return res;
}
- syscall_printf ("%R = select(%d, %p, %p, %p, %p)", res, maxfds, readfds,
- writefds, exceptfds, to);
- return res;
+ __except (EFAULT) {}
+ __endtry
+ return -1;
+}
+
+/* select() is just a wrapper on pselect(). */
+extern "C" int
+cygwin_select (int maxfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
+ struct timeval *to)
+{
+ struct timespec ts;
+ if (to)
+ {
+ ts.tv_sec = to->tv_sec;
+ ts.tv_nsec = to->tv_usec * 1000;
+ }
+ return pselect (maxfds, readfds, writefds, exceptfds,
+ to ? &ts : NULL, NULL);
}
/* This function is arbitrarily split out from cygwin_select to avoid odd
@@ -127,13 +153,13 @@ cygwin_select (int maxfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
for the sel variable. */
static int
select (int maxfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
- DWORD ms)
+ LONGLONG us)
{
- select_stuff::wait_states wait_state = select_stuff::select_loop;
+ select_stuff::wait_states wait_state = select_stuff::select_set_zero;
int ret = 0;
/* Record the current time for later use. */
- LONGLONG start_time = gtod.msecs ();
+ LONGLONG start_time = gtod.usecs ();
select_stuff sel;
sel.return_on_signal = 0;
@@ -155,65 +181,37 @@ select (int maxfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
}
select_printf ("sel.always_ready %d", sel.always_ready);
- /* Degenerate case. No fds to wait for. Just wait for time to run out
- or signal to arrive. */
- if (sel.start.next == NULL)
- switch (cygwait (ms))
- {
- case WAIT_SIGNALED:
- select_printf ("signal received");
- /* select() is always interrupted by a signal so set EINTR,
- unconditionally, ignoring any SA_RESTART detection by
- call_signal_handler(). */
- _my_tls.call_signal_handler ();
- set_sig_errno (EINTR);
- wait_state = select_stuff::select_signalled;
- break;
- case WAIT_CANCELED:
- sel.destroy ();
- pthread::static_cancel_self ();
- /*NOTREACHED*/
- default:
- /* Set wait_state to zero below. */
- wait_state = select_stuff::select_set_zero;
- break;
- }
- else if (sel.always_ready || ms == 0)
+ if (sel.always_ready || us == 0)
/* Catch any active fds via sel.poll() below */
wait_state = select_stuff::select_ok;
else
/* wait for an fd to become active or time out */
- wait_state = sel.wait (r, w, e, ms);
+ wait_state = sel.wait (r, w, e, us);
select_printf ("sel.wait returns %d", wait_state);
- if (wait_state >= select_stuff::select_ok)
+ if (wait_state == select_stuff::select_ok)
{
UNIX_FD_ZERO (readfds, maxfds);
UNIX_FD_ZERO (writefds, maxfds);
UNIX_FD_ZERO (exceptfds, maxfds);
- if (wait_state == select_stuff::select_set_zero)
- ret = 0;
- else
- {
- /* Set bit mask from sel records. This also sets ret to the
- right value >= 0, matching the number of bits set in the
- fds records. if ret is 0, continue to loop. */
- ret = sel.poll (readfds, writefds, exceptfds);
- if (!ret)
- wait_state = select_stuff::select_loop;
- }
+ /* Set bit mask from sel records. This also sets ret to the
+ right value >= 0, matching the number of bits set in the
+ fds records. if ret is 0, continue to loop. */
+ ret = sel.poll (readfds, writefds, exceptfds);
+ if (!ret)
+ wait_state = select_stuff::select_set_zero;
}
/* Always clean up everything here. If we're looping then build it
all up again. */
sel.cleanup ();
sel.destroy ();
- /* Recalculate time remaining to wait if we are going to be looping. */
- if (wait_state == select_stuff::select_loop && ms != INFINITE)
+ /* Check and recalculate timeout. */
+ if (us != -1LL && wait_state == select_stuff::select_set_zero)
{
- select_printf ("recalculating ms");
- LONGLONG now = gtod.msecs ();
- if (now > (start_time + ms))
+ select_printf ("recalculating us");
+ LONGLONG now = gtod.usecs ();
+ if (now > (start_time + us))
{
select_printf ("timed out after verification");
/* Set descriptor bits to zero per POSIX. */
@@ -225,46 +223,19 @@ select (int maxfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
}
else
{
- ms -= (now - start_time);
+ us -= (now - start_time);
start_time = now;
- select_printf ("ms now %u", ms);
+ select_printf ("us now %lld", us);
}
}
}
- while (wait_state == select_stuff::select_loop);
+ while (wait_state == select_stuff::select_set_zero);
if (wait_state < select_stuff::select_ok)
ret = -1;
return ret;
}
-extern "C" int
-pselect(int maxfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
- const struct timespec *ts, const sigset_t *set)
-{
- struct timeval tv;
- sigset_t oldset = _my_tls.sigmask;
-
- __try
- {
- if (ts)
- {
- tv.tv_sec = ts->tv_sec;
- tv.tv_usec = ts->tv_nsec / 1000;
- }
- if (set)
- set_signal_mask (_my_tls.sigmask, *set);
- int ret = cygwin_select (maxfds, readfds, writefds, exceptfds,
- ts ? &tv : NULL);
- if (set)
- set_signal_mask (_my_tls.sigmask, oldset);
- return ret;
- }
- __except (EFAULT) {}
- __endtry
- return -1;
-}
-
/* Call cleanup functions for all inspected fds. Gets rid of any
executing threads. */
void
@@ -362,13 +333,50 @@ err:
/* The heart of select. Waits for an fd to do something interesting. */
select_stuff::wait_states
select_stuff::wait (fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
- DWORD ms)
+ LONGLONG us)
{
HANDLE w4[MAXIMUM_WAIT_OBJECTS];
select_record *s = &start;
DWORD m = 0;
+ /* Always wait for signals. */
wait_signal_arrived here (w4[m++]);
+
+ /* Set a timeout, or not, for WMFO. */
+ DWORD dTimeoutMs;
+ if (us == 0)
+ {
+ dTimeoutMs = 0;
+ }
+ else
+ {
+ dTimeoutMs = INFINITE;
+ }
+ /* Create and set a waitable timer, if a finite timeout has been
+ requested. */
+ LARGE_INTEGER liTimeout;
+ HANDLE hTimeout;
+ NTSTATUS status;
+ status = NtCreateTimer(&hTimeout, TIMER_ALL_ACCESS, NULL, NotificationTimer);
+ if (!NT_SUCCESS (status))
+ {
+ select_printf("NtCreateTimer failed (%d)\n", GetLastError());
+ return select_error;
+ }
+ w4[m++] = hTimeout;
+ if (us >= 0)
+ {
+ liTimeout.QuadPart = -us * 10;
+ int setret;
+ status = NtSetTimer (hTimeout, &liTimeout, NULL, NULL, FALSE, 0, NULL);
+ if (!NT_SUCCESS(status))
+ {
+ select_printf ("NtSetTimer failed: %d (%08x)\n", setret, GetLastError());
+ return select_error;
+ }
+ }
+
+ /* Optionally wait for pthread cancellation. */
if ((w4[m] = pthread::get_cancel_event ()) != NULL)
m++;
@@ -397,21 +405,27 @@ select_stuff::wait (fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
next_while:;
}
- debug_printf ("m %d, ms %u", m, ms);
+ debug_printf ("m %d, us %llu, dTimeoutMs %d", m, us, dTimeoutMs);
DWORD wait_ret;
if (!windows_used)
- wait_ret = WaitForMultipleObjects (m, w4, FALSE, ms);
+ wait_ret = WaitForMultipleObjects (m, w4, FALSE, dTimeoutMs);
else
/* Using MWMO_INPUTAVAILABLE is the officially supported solution for
the problem that the call to PeekMessage disarms the queue state
so that a subsequent MWFMO hangs, even if there are still messages
in the queue. */
- wait_ret = MsgWaitForMultipleObjectsEx (m, w4, ms,
+ wait_ret = MsgWaitForMultipleObjectsEx (m, w4, dTimeoutMs,
QS_ALLINPUT | QS_ALLPOSTMESSAGE,
MWMO_INPUTAVAILABLE);
select_printf ("wait_ret %d, m = %d. verifying", wait_ret, m);
+ if (dTimeoutMs == INFINITE)
+ {
+ CancelWaitableTimer (hTimeout);
+ CloseHandle (hTimeout);
+ }
+
wait_states res;
switch (wait_ret)
{
@@ -434,12 +448,13 @@ next_while:;
s->set_select_errno ();
res = select_error;
break;
+ case WAIT_OBJECT_0 + 1:
case WAIT_TIMEOUT:
select_printf ("timed out");
res = select_set_zero;
break;
- case WAIT_OBJECT_0 + 1:
- if (startfds > 1)
+ case WAIT_OBJECT_0 + 2:
+ if (startfds > 2)
{
cleanup ();
destroy ();
@@ -450,7 +465,7 @@ next_while:;
to wait for. */
default:
s = &start;
- bool gotone = false;
+ res = select_set_zero;
/* Some types of objects (e.g., consoles) wake up on "inappropriate"
events like mouse movements. The verify function will detect these
situations. If it returns false, then this wakeup was a false alarm
@@ -464,13 +479,9 @@ next_while:;
}
else if ((((wait_ret >= m && s->windows_handle) || s->h == w4[wait_ret]))
&& s->verify (s, readfds, writefds, exceptfds))
- gotone = true;
+ res = select_ok;
- if (!gotone)
- res = select_loop;
- else
- res = select_ok;
- select_printf ("gotone %d", gotone);
+ select_printf ("res after verify %d", res);
break;
}
out:
@@ -839,7 +850,6 @@ fhandler_fifo::select_except (select_stuff *ss)
static int
peek_console (select_record *me, bool)
{
- extern const char * get_nonascii_key (INPUT_RECORD& input_rec, char *);
fhandler_console *fh = (fhandler_console *) me->fh;
if (!me->read_selected)
@@ -875,19 +885,29 @@ peek_console (select_record *me, bool)
{
if (irec.Event.KeyEvent.bKeyDown
&& (irec.Event.KeyEvent.uChar.AsciiChar
- || get_nonascii_key (irec, tmpbuf)))
- return me->read_ready = true;
+ || fhandler_console::get_nonascii_key (irec, tmpbuf)))
+ {
+ debug_printf("peeked KEY_EVENT");
+ return me->read_ready = true;
+ }
}
else
{
if (irec.EventType == MOUSE_EVENT
&& fh->mouse_aware (irec.Event.MouseEvent))
+ {
+ debug_printf("peeked MOUSE_EVENT");
return me->read_ready = true;
+ }
if (irec.EventType == FOCUS_EVENT && fh->focus_aware ())
+ {
+ debug_printf("peeked FOCUS_EVENT");
return me->read_ready = true;
+ }
}
/* Read and discard the event */
+ debug_printf("discarded other event");
ReadConsoleInput (h, &irec, 1, &events_read);
}
diff --git a/winsup/cygwin/select.h b/winsup/cygwin/select.h
index 0035820..3c749ad 100644
--- a/winsup/cygwin/select.h
+++ b/winsup/cygwin/select.h
@@ -78,7 +78,6 @@ public:
enum wait_states
{
select_signalled = -3,
- select_loop = -2,
select_error = -1,
select_ok = 0,
select_set_zero = 1
@@ -96,7 +95,7 @@ public:
bool test_and_set (int, fd_set *, fd_set *, fd_set *);
int poll (fd_set *, fd_set *, fd_set *);
- wait_states wait (fd_set *, fd_set *, fd_set *, DWORD);
+ wait_states wait (fd_set *, fd_set *, fd_set *, LONGLONG);
void cleanup ();
void destroy ();
diff --git a/winsup/testsuite/configure b/winsup/testsuite/configure
old mode 100644
new mode 100755
--
2.7.2
More information about the Cygwin-patches
mailing list