Hi Takashi, On Mar 12 12:27, Takashi Yano wrote: > The previous implementation of the signal queue behaves as: > 1) Signals in the queue are processed in a disordered manner. > 2) If the same signal is already in the queue, new signal is discarded. > > Strictly speaking, these behaviours do not violate POSIX. However, > these could be a cause of unexpected behaviour in some software. In > Linux, some important signals such as SIGSTOP/SIGCONT do not seem to > behave like that. > This patch prevents SIGKILL, SIGSTOP, SIGCONT, and SIGRT* from that > issue. Moreover, if SA_SIGINFO is set in sa_flags, the signal is > treated almost as the same. > > Addresses: https://cygwin.com/pipermail/cygwin/2025-March/257582.html > Fixes: 7ac6173643b1 ("(pending_signals): New class.") > Reported by: Christian Franke <christian.fra...@t-online.de> > Reviewed-by: Corinna Vinschen <cori...@vinschen.de>, Christian Franke > <christian.fra...@t-online.de> > Signed-off-by: Takashi Yano <takashi.y...@nifty.ne.jp> > --- > winsup/cygwin/sigproc.cc | 128 ++++++++++++++++++++++++++++++++------- > 1 file changed, 106 insertions(+), 22 deletions(-) > > diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc > index 8739f18f5..ab3acfd24 100644 > --- a/winsup/cygwin/sigproc.cc > +++ b/winsup/cygwin/sigproc.cc > @@ -21,6 +21,7 @@ details. */ > #include "cygtls.h" > #include "ntdll.h" > #include "exception.h" > +#include <assert.h> > > /* > * Convenience defines > @@ -28,6 +29,10 @@ details. */ > #define WSSC 60000 // Wait for signal completion > #define WPSP 40000 // Wait for proc_subproc mutex > > +#define PIPE_DEPTH _NSIG /* Historically, the pipe size is _NSIG packet */ > +#define SIGQ_ROOM 4 > +#define SIGQ_DEPTH (PIPE_DEPTH + SIGQ_ROOM)
I'm missing a comment here. Why adding SIGQ_ROOM? Other than that, LGTM. In terms of the queue size, I wonder if we really have to restrict the queue to a small number of queued signals, 69 right now. The pipe used for communication will take 64K, one allocation granularity slot, anyway. Linux, for instance, queues more than 60K signals. So, wouldn't it make sense to raise the queu depth to some higher value and the pipe size so that it it's <= 64K? While looking into your patch, it occured to me that we have a long-standing bug: We never changed __SIGQUEUE_MAX/SIGQUEUE_MAX in include/cygwin/limits.h when we started to support 64 signals (we only did that for 64 bit Cygwin). We can't change that for existing binaries actually referring the SIGQUEUE_MAX macro, but we should change this, so that sysconf( _SC_SIGQUEUE_MAX) returns the right value, isn't it? Thanks, Corinna
