* DevNotes: Add entry cgf-000016.

* cygtls.h (_cygtls::push): Inline.
(_cygtls::interrupt_now): Change signal number argument to siginfo_t argument.
(_cygtls::interrupt_setup): Ditto.
(_cygtls::set_siginfo): Delete declaration.
(_cygtls::reset_signal_arrived): Don't reset signal_arrived signal.  Just reset
flag.
* exceptions.cc (_cygtls::interrupt_now): Reflect argument changes.  Pass si to
interrupt_setup.
(_cygtls::interrupt_setup): Reflect argument changes.  Fill out tls infodata
here using passed-in si.  Use si.si_signo instead of sig.
(sigpacket::setup_handler): Move this function into sigpacket class.  Use si
field from the class as appropriate.
(sigpacket::process): Don't call tls->set_siginfo here since setup_handler
could fail.  Eliminate now-unneeded sig argument.
* sigproc.h (sigpacket::setup_handler): Move setup_handler to this class.
This commit is contained in:
Christopher Faylor 2012-08-17 17:29:21 +00:00
parent 23338be7f5
commit 39d0171500
6 changed files with 66 additions and 38 deletions

View File

@ -1,3 +1,24 @@
2012-08-17 Christopher Faylor <me.cygwin2012@cgf.cx>
* DevNotes: Add entry cgf-000016.
* cygtls.h (_cygtls::push): Inline.
(_cygtls::interrupt_now): Change signal number argument to siginfo_t
argument.
(_cygtls::interrupt_setup): Ditto.
(_cygtls::set_siginfo): Delete declaration.
(_cygtls::reset_signal_arrived): Don't reset signal_arrived signal.
Just reset flag.
* exceptions.cc (_cygtls::interrupt_now): Reflect argument changes.
Pass si to interrupt_setup.
(_cygtls::interrupt_setup): Reflect argument changes. Fill out tls
infodata here using passed-in si. Use si.si_signo instead of sig.
(sigpacket::setup_handler): Move this function into sigpacket class.
Use si field from the class as appropriate.
(sigpacket::process): Don't call tls->set_siginfo here since
setup_handler could fail. Eliminate now-unneeded sig argument.
* sigproc.h (sigpacket::setup_handler): Move setup_handler to this
class.
2012-08-17 Christopher Faylor <me.cygwin2012@cgf.cx>
* exceptions.cc (sig_handle_tty_stop): Clear tls sig field.

View File

@ -1,3 +1,29 @@
2012-08-17 cgf-000016
While debugging another problem I finally noticed that
sigpacket::process was unconditionally calling tls->set_siginfo prior to
calling setup_handler even though setup_handler could fail. In the
event of two successive signals, that would cause the second signal's
info to overwrite the first even though the signal handler for the first
would eventually be called. Doh.
Fixing this required passing the sigpacket si field into setup_handler.
Making setup_handler part of the sigpacket class seemed to make a lot of
sense so that's what I did. Then I passed the si element into
interrupt_setup so that the infodata structure could be filled out prior
to arming the signal.
The other changes checked in here eliminate the ResetEvent for
signal_arrived since previous changes to cygwait should handle the
case of spurious signal_arrived detection. Since signal_arrived is
not a manual-reset event, we really should just let the appropriate
WFMO handle it. Otherwise, there is a race where a signal comes in
a "split second" after WFMO responds to some other event. Resetting
the signal_arrived would cause any subsequent WFMO to never be
triggered. My current theory is that this is what is causing:
http://cygwin.com/ml/cygwin/2012-08/msg00310.html
2012-08-15 cgf-000015
RIP cancelable_wait. Yay.

View File

@ -196,15 +196,3 @@ _cygtls::remove (DWORD wait)
cygheap->remove_tls (this, wait);
remove_wq (wait);
}
void
_cygtls::push (__stack_t addr)
{
*stackptr++ = (__stack_t) addr;
}
void
_cygtls::set_siginfo (sigpacket *pack)
{
infodata = pack->si;
}

View File

@ -207,17 +207,16 @@ public:
void init_thread (void *, DWORD (*) (void *, void *));
static void call (DWORD (*) (void *, void *), void *);
void remove (DWORD);
void push (__stack_t) __attribute__ ((regparm (2)));
void push (__stack_t addr) {*stackptr++ = (__stack_t) addr;}
__stack_t pop () __attribute__ ((regparm (1)));
__stack_t retaddr () {return stackptr[-1];}
bool isinitialized () const
{
return initialized == CYGTLS_INITIALIZED;
}
bool interrupt_now (CONTEXT *, int, void *, struct sigaction&)
bool interrupt_now (CONTEXT *, siginfo_t&, void *, struct sigaction&)
__attribute__((regparm(3)));
void __stdcall interrupt_setup (int sig, void *handler,
struct sigaction& siga)
void __stdcall interrupt_setup (siginfo_t&, void *, struct sigaction&)
__attribute__((regparm(3)));
bool inside_kernel (CONTEXT *);
@ -228,7 +227,6 @@ public:
#ifdef CYGTLS_HANDLE
operator HANDLE () const {return tid ? tid->win32_obj_id : NULL;}
#endif
void set_siginfo (struct sigpacket *) __attribute__ ((regparm (3)));
int call_signal_handler () __attribute__ ((regparm (1)));
void remove_wq (DWORD) __attribute__ ((regparm (1)));
void fixup_after_fork () __attribute__ ((regparm (1)));
@ -255,12 +253,7 @@ public:
signal_waiting = true;
}
}
void reset_signal_arrived ()
{
if (signal_arrived)
ResetEvent (signal_arrived);
signal_waiting = false;
}
void reset_signal_arrived () { signal_waiting = false; }
private:
void call2 (DWORD (*) (void *, void *), void *, void *) __attribute__ ((regparm (3)));
/*gentls_offsets*/

View File

@ -757,7 +757,7 @@ sig_handle_tty_stop (int sig)
} /* end extern "C" */
bool
_cygtls::interrupt_now (CONTEXT *cx, int sig, void *handler,
_cygtls::interrupt_now (CONTEXT *cx, siginfo_t& si, void *handler,
struct sigaction& siga)
{
bool interrupted;
@ -771,7 +771,7 @@ _cygtls::interrupt_now (CONTEXT *cx, int sig, void *handler,
else
{
push ((__stack_t) cx->Eip);
interrupt_setup (sig, handler, siga);
interrupt_setup (si, handler, siga);
cx->Eip = pop ();
SetThreadContext (*this, cx); /* Restart the thread in a new location */
interrupted = true;
@ -780,7 +780,7 @@ _cygtls::interrupt_now (CONTEXT *cx, int sig, void *handler,
}
void __stdcall
_cygtls::interrupt_setup (int sig, void *handler, struct sigaction& siga)
_cygtls::interrupt_setup (siginfo_t& si, void *handler, struct sigaction& siga)
{
push ((__stack_t) sigdelayed);
deltamask = siga.sa_mask & ~SIG_NONMASKABLE;
@ -795,7 +795,8 @@ _cygtls::interrupt_setup (int sig, void *handler, struct sigaction& siga)
myself->process_state |= PID_STOPPED;
}
this->sig = sig; // Should always be last thing set to avoid a race
infodata = si;
this->sig = si.si_signo; // Should always be last thing set to avoid a race
if (incyg)
{
@ -805,7 +806,7 @@ _cygtls::interrupt_setup (int sig, void *handler, struct sigaction& siga)
}
proc_subproc (PROC_CLEARWAIT, 1);
sigproc_printf ("armed signal_arrived %p, signal %d", signal_arrived, sig);
sigproc_printf ("armed signal_arrived %p, signal %d", signal_arrived, si.si_signo);
}
extern "C" void __stdcall
@ -815,10 +816,8 @@ set_sig_errno (int e)
_my_tls.saved_errno = e;
}
static int setup_handler (int, void *, struct sigaction&, _cygtls *tls)
__attribute__((regparm(3)));
static int
setup_handler (int sig, void *handler, struct sigaction& siga, _cygtls *tls)
int
sigpacket::setup_handler (void *handler, struct sigaction& siga, _cygtls *tls)
{
CONTEXT cx;
bool interrupted = false;
@ -826,7 +825,7 @@ setup_handler (int sig, void *handler, struct sigaction& siga, _cygtls *tls)
if (tls->sig)
{
sigproc_printf ("trying to send signal %d but signal %d already armed",
sig, tls->sig);
si.si_signo, tls->sig);
goto out;
}
@ -839,7 +838,7 @@ setup_handler (int sig, void *handler, struct sigaction& siga, _cygtls *tls)
{
sigproc_printf ("controlled interrupt. stackptr %p, stack %p, stackptr[-1] %p",
tls->stackptr, tls->stack, tls->stackptr[-1]);
tls->interrupt_setup (sig, handler, siga);
tls->interrupt_setup (si, handler, siga);
interrupted = true;
tls->unlock ();
goto out;
@ -868,7 +867,7 @@ setup_handler (int sig, void *handler, struct sigaction& siga, _cygtls *tls)
if (!GetThreadContext (hth, &cx))
sigproc_printf ("couldn't get context of thread, %E");
else
interrupted = tls->interrupt_now (&cx, sig, handler, siga);
interrupted = tls->interrupt_now (&cx, si, handler, siga);
tls->unlock ();
ResumeThread (hth);
@ -885,7 +884,7 @@ setup_handler (int sig, void *handler, struct sigaction& siga, _cygtls *tls)
}
out:
sigproc_printf ("signal %d %sdelivered", sig, interrupted ? "" : "not ");
sigproc_printf ("signal %d %sdelivered", si.si_signo, interrupted ? "" : "not ");
return interrupted;
}
@ -1234,10 +1233,9 @@ dosig:
rc = -1; /* No signals delivered if stopped */
else
{
tls->set_siginfo (this);
/* Dispatch to the appropriate function. */
sigproc_printf ("signal %d, signal handler %p", si.si_signo, handler);
rc = setup_handler (si.si_signo, handler, thissig, tls);
rc = setup_handler (handler, thissig, tls);
continue_now = false;
}

View File

@ -56,6 +56,8 @@ struct sigpacket
struct sigpacket *next;
};
int __stdcall process () __attribute__ ((regparm (1)));
int setup_handler (void *handler, struct sigaction& siga, _cygtls *tls)
__attribute__ ((regparm (3)));
};
void __stdcall sig_dispatch_pending (bool fast = false)