<br><br><div class="gmail_quote">On Wed, Oct 12, 2011 at 3:56 PM, Thomas Cataldo <span dir="ltr"><<a href="mailto:tcataldo@gmail.com">tcataldo@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br><br><div class="gmail_quote"><div><div></div><div class="h5">On Wed, Oct 12, 2011 at 3:48 PM, Henrique de Moraes Holschuh <span dir="ltr"><<a href="mailto:hmh@debian.org" target="_blank">hmh@debian.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>On Tue, 11 Oct 2011, Greg Banks wrote:<br>
> On 11/10/11 16:57, Bron Gondwana wrote:<br>
> > On Tue, Oct 11, 2011 at 02:51:25AM +0200, Thomas Cataldo wrote:<br>
> >> While testing some java code that executes cyrus init script, I came into a<br>
> >> problem : the Java VM blocks SIGQUIT, event when using -Xrs parameter.<br>
<br>
</div>[...]<br>
<div><br>
> >> --- a/master/master.c<br>
> >> +++ b/master/master.c<br>
> >> @@ -1064,7 +1064,11 @@ void sigalrm_handler(int sig __attribute__((unused)))<br>
> >> void sighandler_setup(void)<br>
> >> {<br>
> >> struct sigaction action;<br>
> >> -<br>
> >> + sigset_t all_signals;<br>
> >> +<br>
> >> + sigfillset(&all_signals);<br>
> >> + sigprocmask(SIG_UNBLOCK, &all_signals, NULL);<br>
> >> +<br>
> >> sigemptyset(&action.sa_mask);<br>
> >> action.sa_flags = 0;<br>
<br>
</div>[...]<br>
<div><br>
> FWIW I don't see any point explicitly unblocking all the signals,<br>
> including ones other than the ones we're about to explicitly setup<br>
> handlers for, but that's probably harmless.<br>
<br>
</div>IMHO we're better off unblocking just what we'll handle, better safe than<br>
sorry. If we're not going to handle it and we don't expect it, better to<br>
never unblock them.<br>
<br>
In fact, if there are signals we _want_ blocked, it is best to explicitly do<br>
just that as well.<br>
<div><br>
> Also, as a defensive programming measure it's probably a good idea to<br>
> memset() that struct sigaction to zero rather than explicitly initialise<br>
> some of its members.<br>
<br>
</div>Agreed.<br>
<br>
Thomas, could you respin the patch to only unblock the interesting<br>
signals, and zero-fill struct sigaction?<br></blockquote><div><br></div></div></div><div>Ok I'll adjust that tonight & resubmit a version that only unblocks the signals cyrmaster handles.</div><div><br></div><div>
Thanks for your feedback.</div>
<div><br></div><div><br></div></div>
</blockquote></div><br><div>This one should fit your expectations : memset & only unblock signals we had handlers to.</div><div><br></div><div><div>diff --git a/master/master.c b/master/master.c</div><div>index 8847255..fa1b40b 100644</div>
<div>--- a/master/master.c</div><div>+++ b/master/master.c</div><div>@@ -1076,10 +1076,17 @@ static void sigalrm_handler(int sig __attribute__((unuse</div><div> static void sighandler_setup(void)</div><div> {</div><div> struct sigaction action;</div>
<div>- sigset_t all_signals;</div><div>-</div><div>- sigfillset(&all_signals);</div><div>- sigprocmask(SIG_UNBLOCK, &all_signals, NULL);</div><div>+ sigset_t siglist;</div><div>+</div><div>+ memset(&siglist, 0, sizeof(siglist));</div>
<div>+ sigemptyset(&siglist);</div><div>+ sigaddset(&siglist, SIGHUP);</div><div>+ sigaddset(&siglist, SIGALRM);</div><div>+ sigaddset(&siglist, SIGQUIT);</div><div>+ sigaddset(&siglist, SIGTERM);</div>
<div>+ sigaddset(&siglist, SIGINT);</div><div>+ sigaddset(&siglist, SIGCHLD);</div><div>+ sigprocmask(SIG_UNBLOCK, &siglist, NULL);</div><div> </div><div> sigemptyset(&action.sa_mask);</div><div>
action.sa_flags = 0;</div></div><div><br></div><div><br></div><div>Only compile tested on head. I normally test using git://<a href="http://git.debian.org/git/pkg-cyrus-imapd/cyrus-imapd-2.4.git">git.debian.org/git/pkg-cyrus-imapd/cyrus-imapd-2.4.git</a> but I keep on getting quilt rejects (and I don't know how to use quilt)</div>
<div><br></div><div>Regards,</div><div>Tom.</div><div><br></div><div><br></div>