This is the mail archive of the cygwin-patches@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Signal handling tune up.


During my work on using the multimedia timer for setitimer I noticed
that Cygwin was not optimized to handle 1000 signals per second.
It starts trashing under heavy load. Attached is a tune up patch.
It also modestly improves performances at light load and fixes a
race condition.
   
Some explanations are in order, here they are.

************
1)
Note the pattern of frames on the stack
  220 1565622 [sig] a 34108167 interruptible: pc 0x6100FCFB, h 0x61000000,
interruptible 0, testvalid 0
  227 1565849 [sig] a 34108167 interruptible: pc 0x61010700, h 0x61000000,
interruptible 0, testvalid 0
  222 1566071 [sig] a 34108167 interruptible: pc 0x610105E6, h 0x61000000,
interruptible 0, testvalid 0

<snip> <snip>   

  226 1576965 [sig] a 34108167 interruptible: pc 0x610105E6, h 0x61000000,
interruptible 0, testvalid 0
  221 1577186 [sig] a 34108167 interruptible: pc 0x6101070E, h 0x61000000,
interruptible 0, testvalid 0
 1840 1579026 [sig] a 34108167 interrupt_on_return: couldn't find a stack
frame, i 32

~: addr2line -e /bin/cygwin1.dll  -f 0x610105E6
call_signal_handler_now
/eroot/obj/i586-pc-cygwin/winsup/cygwin/../../../../src/winsup/cygwin/except
ions.cc:1188
~: addr2line -e /bin/cygwin1.dll  -f 0x6101070e
unused_sig_wrapper
/eroot/obj/i586-pc-cygwin/winsup/cygwin/../../../../src/winsup/cygwin/except
ions.cc:1221

What's happening is that when a signal handler terminates and another signal 
is pending (which is often the case in heavy load), Cygwin immediately
starts a 
new one, expanding the stack in the process. Scanning the stack takes time,
and
when the limit of 32 is reached drastic actions are taken.
Fixed with minor assembly changes  

*********************************************************************
2)
        movl    $0,%0                   # zero the signal number as a   \n\
                                        # flag to the signal handler thread\n\
                                        # that it is ok to set up sigsave\n\
                                                                        \n\
       call    _set_process_mask@4
There is a race where the sigthread can start a handler for a signal that
should be blocked.
Simply interchanging the order still allows the sigthread to try to launch 
a handler (before the mask is set), discovers that sigsave is busy and takes 
cumbersome actions (e.g. Sleep).
The patch moves set_process_mask all the way up to interrupt_setup(), so
it is set in the sigthread itself.

*********************************************************************
3)
Posix says "If a subsequent occurrence of a pending signal is generated,
it is implementation-defined as to whether the signal is delivered or 
accepted more than once [RTS]   in circumstances other than those in 
which queuing is required under the Realtime Signals Extension option".

Cygwin takes the "more than once" path and increments sigtodo without
bound. That's fine, except in the case of periodic exceptions that occur
faster than they can be processed. There they should better be ignored.
FWIW, that's a Posix requirement for timer_settime().

So, my (to come) implementation of setitimer examines sigtodo before 
generating an exception. To make this work I had to modify somewhat 
InterlockedDecrement/Decrement (myself->getsigtodo (sig)) to make sure
that it always has the correct value.

**********************************************************************
4) 
What to do when an interrupt cannot be delivered has been modified to
minimize delays. This went together with fixing (?) FIXMEs in 
setup_handler () and wait_sig(). See the saw_failed_interrupt stuff.


There are also some minor and more obvious changes. 

**********************************************************************
5) 
This is just an observation, about sig_handle (int sig, bool thisproc)
The thisproc argument is set to "rc != 2" in the sigthread. However
it is possible for several signals to occur simultaneously and a
signal can be processed with rc == 2 even when generated by the 
current process (or conversely). This could cause a SIGINT to be 
ignored (or not) when it shouldn't (something like that was discussed
on the list recently).
I don't fully understand the use of thisproc and have no suggestion. 

 
Fortunately the ChangeLog is much shorter than the explanations above.


2003-08-18  Pierre Humblet <pierre.humblet@ieee.org>

	* exceptions.cc (interrupt_setup): Decrement sigtodo and set the mask.
	(set_process_mask): Don't signal sigthread if old mask is more permissive.
	(sig_handle): Return -1 when no handler function is called.
	(unused_sig_wrapper): In sigreturn, handle the eventual new signal 
	without growing the stack. In sigdelayed, do not call set_process_mask.
	* sigproc.cc (sig_send): Set pending_signals.
	(sig_set_pending): Delete.
	(wait_sig): Change the way of changing sigtodo and of handling failed
	interrupts. Modify some sigproc_printf().

Attachment: signal.diff
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]