On Tue, 3 Dec 2024 21:39:33 +0900
Takashi Yano wrote:
> On Mon, 2 Dec 2024 16:18:15 +0100
> Corinna Vinschen wrote:
> > On Nov 29 20:59, Takashi Yano wrote:
> > > Previously, the sig thread ran in THREAD_PRIORITY_HIGHEST priority.
> > > This causes a critical delay in the signal handling in the main
> > > thread if too many signals are received rapidly and the CPU is very
> > > busy. In this case, most of the CPU time is allocated to the sig
> > > thread, so the main thread cannot have a chance of handling signals.
> > > With this patch, to avoid such a situation, the priority of the sig
> > > thread is set to THREAD_PRIORITY_NORMAL priority. Furthermore, if
> > > the signal is alerted to the main thread, but the main thread does
> > > not handle it yet, to increase the chance of handling it in the main
> > > thread, reduce the sig thread priority to THREAD_PRIORITY_LOWEST
> > > priority temporarily before calling _cygtls::handle_SIGCONT().
> > >
> > > Addresses: https://cygwin.com/pipermail/cygwin/2024-November/256744.html
> > > Fixes: 53ad6f1394aa ("(cygthread::cygthread): Use three only arguments
> > > for detached threads, and start the thread via
> > > QueueUserAPC/async_create.")
> > > Reported-by: Christian Franke <christian.fra...@t-online.de>
> > > Reviewed-by: Corinna Vinschen <cori...@vinschen.de>
> > > Signed-off-by: Takashi Yano <takashi.y...@nifty.ne.jp>
> > > ---
> > > winsup/cygwin/exceptions.cc | 6 ++++++
> > > winsup/cygwin/sigproc.cc | 1 +
> > > 2 files changed, 7 insertions(+)
> > >
> > > diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
> > > index 0f8c21939..7fc644af1 100644
> > > --- a/winsup/cygwin/exceptions.cc
> > > +++ b/winsup/cygwin/exceptions.cc
> > > @@ -978,6 +978,9 @@ sigpacket::setup_handler (void *handler, struct
> > > sigaction& siga, _cygtls *tls)
> > > CONTEXT cx;
> > > bool interrupted = false;
> > >
> > > + for (int i = 0; i < 100 && tls->current_sig; i++)
> > > + yield ();
> > > +
> >
> > Is that a piece of stray code left from testing, or is that actually
> > part of the patch? The commit message doesn't explain it...
>
> Oops! Sorry, this is test code for another patch I tested now.
> I'll submit v4 patch as well as another patch signal-related.
With the patch ("another patch" above):
"Cygwin: signal: Increase chance of handling signal in main thread"
it seems that we do not need setting thread priority around
handle_SIGCONT() any more. I cannot explain why though...
--
Takashi Yano <takashi.y...@nifty.ne.jp>
