Re: [PATCH] Cygwin: signal: Fix high load when retrying to process pending signal

[Corinna Vinschen]

On Dec 12 17:32, Takashi Yano wrote:
> The commit e10f822a2b39 has a problem that CPU load gets high if
> pending signal is not processed successfully for a long time.
> With this patch, wait_sig() calls Sleep(1), rather than yield(),
> if the pending signal has not been processed successfully for a
> predetermined time to prevent CPU from high load.
> 
> Addresses: https://cygwin.com/pipermail/cygwin/2024-December/256884.html
> Fixes: e10f822a2b39 ("Cygwin: signal: Handle queued signal without explicit 
> __SIGFLUSH")
> Reported-by: 凯夏 <walke...@gmail.com>
> Reviewed-by:
> Signed-off-by: Takashi Yano <takashi.y...@nifty.ne.jp>
> ---
>  winsup/cygwin/sigproc.cc | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
> index 59b4208a6..e01a67ebe 100644
> --- a/winsup/cygwin/sigproc.cc
> +++ b/winsup/cygwin/sigproc.cc
> @@ -1345,6 +1345,12 @@ wait_sig (VOID *)
>  
>    hntdll = GetModuleHandle ("ntdll.dll");
>  
> +  /* GetTickCount() here is enough because GetTickCount() - t0 does
> +     not overflow until 49 days psss. Even if GetTickCount() overflows,
> +     GetTickCount() - t0 returns correct value, since underflow in
> +     unsigned wraps correctly. Pending a signal for more thtn 49
> +     days would be noncense. */
> +  DWORD t0 = GetTickCount ();
>    for (;;)
>      {
>        DWORD nb;
> @@ -1354,7 +1360,10 @@ wait_sig (VOID *)
>        else if (sigq.start.next
>              && PeekNamedPipe (my_readsig, NULL, 0, NULL, &nb, NULL) && !nb)
>       {
> -       yield ();
> +       if (GetTickCount () - t0 > 10)
> +         Sleep (1);
> +       else
> +         yield ();

Since yield() is the same as Sleep(0), you could also just make
that a one-liner, kind of like this:

          Sleep (GetTickCount () - t0 > 10 ? 1 : 0);

Apart from that, LGTM.

Thanks,
Corinna