On Wed, Oct 12, 2011 at 3:56 PM, Thomas Cataldo <tcata...@gmail.com> wrote:
>
>
> On Wed, Oct 12, 2011 at 3:48 PM, Henrique de Moraes Holschuh <
> h...@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.
signals_rework.diff
Description: Binary data
