[PATCH] signal handlers setup

[Thomas Cataldo]

On Wed, Oct 12, 2011 at 3:56 PM, Thomas Cataldo <tcataldo at gmail.com> wrote:

>
>
> On Wed, Oct 12, 2011 at 3:48 PM, Henrique de Moraes Holschuh <
> hmh at debian.org> wrote:
>
>> On Tue, 11 Oct 2011, Greg Banks wrote:
>> > On 11/10/11 16:57, Bron Gondwana wrote:
>> > > On Tue, Oct 11, 2011 at 02:51:25AM +0200, Thomas Cataldo wrote:
>> > >> While testing some java code that executes cyrus init script, I came
>> into a
>> > >> problem : the Java VM blocks SIGQUIT, event when using -Xrs
>> parameter.
>>
>> [...]
>>
>> > >> --- a/master/master.c
>> > >> +++ b/master/master.c
>> > >> @@ -1064,7 +1064,11 @@ void sigalrm_handler(int sig
>> __attribute__((unused)))
>> > >>  void sighandler_setup(void)
>> > >>  {
>> > >>      struct sigaction action;
>> > >> -
>> > >> +    sigset_t all_signals;
>> > >> +
>> > >> +    sigfillset(&all_signals);
>> > >> +    sigprocmask(SIG_UNBLOCK, &all_signals, NULL);
>> > >> +
>> > >>      sigemptyset(&action.sa_mask);
>> > >>      action.sa_flags = 0;
>>
>> [...]
>>
>> > FWIW I don't see any point explicitly unblocking all the signals,
>> > including ones other than the ones we're about to explicitly setup
>> > handlers for, but that's probably harmless.
>>
>> IMHO we're better off unblocking just what we'll handle, better safe than
>> sorry.  If we're not going to handle it and we don't expect it, better to
>> never unblock them.
>>
>> In fact, if there are signals we _want_ blocked, it is best to explicitly
>> do
>> just that as well.
>>
>> > Also, as a defensive programming measure it's probably a good idea to
>> > memset() that struct sigaction to zero rather than explicitly initialise
>> > some of its members.
>>
>> Agreed.
>>
>> Thomas, could you respin the patch to only unblock the interesting
>> signals, and zero-fill struct sigaction?
>>
>
> Ok I'll adjust that tonight & resubmit a version that only unblocks the
> signals cyrmaster handles.
>
> Thanks for your feedback.
>
>
>
This one should fit your expectations : memset & only unblock signals we had
handlers to.

diff --git a/master/master.c b/master/master.c
index 8847255..fa1b40b 100644
--- a/master/master.c
+++ b/master/master.c
@@ -1076,10 +1076,17 @@ static void sigalrm_handler(int sig
__attribute__((unuse
 static void sighandler_setup(void)
 {
     struct sigaction action;
-    sigset_t all_signals;
-
-    sigfillset(&all_signals);
-    sigprocmask(SIG_UNBLOCK, &all_signals, NULL);
+    sigset_t siglist;
+
+    memset(&siglist, 0, sizeof(siglist));
+    sigemptyset(&siglist);
+    sigaddset(&siglist, SIGHUP);
+    sigaddset(&siglist, SIGALRM);
+    sigaddset(&siglist, SIGQUIT);
+    sigaddset(&siglist, SIGTERM);
+    sigaddset(&siglist, SIGINT);
+    sigaddset(&siglist, SIGCHLD);
+    sigprocmask(SIG_UNBLOCK, &siglist, NULL);

     sigemptyset(&action.sa_mask);
     action.sa_flags = 0;


Only compile tested on head. I normally test using git://
git.debian.org/git/pkg-cyrus-imapd/cyrus-imapd-2.4.git but I keep on getting
quilt rejects (and I don't know how to use quilt)

Regards,
Tom.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20111013/79905ec3/attachment.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signals_rework.diff
Type: application/octet-stream
Size: 799 bytes
Desc: not available
Url : http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20111013/79905ec3/attachment.obj