On Wed, 4 Dec 2024 13:53:14 +0100
Corinna Vinschen wrote:
> On Dec 4 20:41, Takashi Yano wrote:
> > Currently, the signal queue is touched by the thread sig as well as
> > other threads that call sigaction_worker(). This potentially has
> > a possibility to destroy the signal queue chain. A possible worst
> > result may be a self-loop chain which causes infinite loop. With
> > this patch, lock()/unlock() are introduce to avoid such a situation.
>
> I was hoping for a lockfree solution, but never mind that for now.
I cannot imagine how the lockfree can be realized.
Consider the queue chain:
A->B->C->D
A<-B<-C<-D
where "->" shows next link and "<-" shows prev link.
Assume a thread tries to clear B, and another thread tries to clear C.
Two processes, i.e.
clear(B)
B->prev->next = B->next
B->next->prev = B->prev
and
clear(C)
C->prev->next = C->next
C->next->prev = C->prev
are running at the same time. Even if each line is executed atomically,
the next case is not avoidable.
clear(B) clear(C)
B->prev->next = B->next A->next = C
C->prev->next = C->next B->next = D
B->next->prev = B->prev D->prev = A
C->next->prev = C->prev D->prev = B
Then, the result is broken as:
A->C->D
A<-B<-D
B->D
B<-C
while expected result is:
A->D
A<-D
Is there any way to execute atomically two lines:
B->prev->next = B->next
B->next->prev = B->prev
using Interlocked* functions?
> > diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
> > index 7e02e61f7..cc3113b88 100644
> > --- a/winsup/cygwin/sigproc.cc
> > +++ b/winsup/cygwin/sigproc.cc
> > @@ -106,12 +106,27 @@ class pending_signals
> > {
> > sigpacket sigs[_NSIG + 1];
> > sigpacket start;
> > + volatile unsigned locked;
> > bool retry;
> > + void lock ()
> > + {
> > + while (InterlockedExchange (&locked, 1))
> > + {
> > +#ifdef __x86_64__
> > + __asm__ ("pause");
> > +#else
> > +#error unimplemented for this target
> > +#endif
> > + yield ();
> > + }
> > + }
> > + void unlock () { locked = 0; }
> >
> > public:
> > + pending_signals (): locked(0) {}
> > void add (sigpacket&);
> > bool pending () {retry = !!start.next; return retry;}
> > - void clear (int sig);
> > + void clear (int sig, bool need_lock);
> > void clear (_cygtls *tls);
> > friend void sig_dispatch_pending (bool);
> > friend void wait_sig (VOID *arg);
>
> Given this is used in C-code only, what about just using SRWLocks
> in Exclusive-only mode, rather than a spinlock?
I submitted v2 patch. Please check.
--
Takashi Yano <takashi.y...@nifty.ne.jp>
