Blocking SIGTERM in nmbd, why?

[Stefan (metze) Metzmacher]

Jeremy Allison schrieb:
> On Fri, Jan 23, 2009 at 11:22:13AM +0100, Stefan (metze) Metzmacher wrote:
>> Hi Jeremy,
>>
>> why do we block SIGTERM in nmbd most of the time?
>> You added it in 1997, see
>> http://gitweb.samba.org/?p=samba.git;a=commitdiff;h=d98bea900ee694cdba83149620c65bd7f8765f26
>>
>> The only reason I can see for this is that the sig_term() function,
>> really does the shutdown directly at this time, see
>> http://gitweb.samba.org/?p=samba.git;a=blob;f=source3/nmbd/nmbd.c;h=dc3f642cd6074944de0eb491e4e70c18616c6fc1;hb=d98bea900ee694cdba83149620c65bd7f8765f26
> 
> Yes, that sounds familiar. The goal of blocking SIGTERM
> and others was that would only take signals in the select()
> statement, blocking them at all other times so that we'd
> not have to deal with signal-safe system call issues such
> as having send() or read() return EINTR.
> 
> This was one of my earlier attempts to deal with the problem
> of non-signal safe code elsewhere in Samba. We're much
> more aware of signal issues now, so this might be ok to
> remove.
> 
>> But now that the signal handler only sets the do_sigterm flag,
>> I can't see the reason why we should still block SIGTERM.
>>
>> I think removing the signal handler temporary when we do
>> the interface detection loop is ok for now. But we
>> should not block SIGTERM the rest of the time.
>>
>> Please review this commits:
>> http://gitweb.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=3aa3a8b6274a11cf27a2df4df97fe011b8b5aa76
>> http://gitweb.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=0860632d25c19dc111fce5b71d6517fafedf9ec1
> 
> Parts of nmbd are still not signal-safe. Look
> at the sendto() call in send_udp() in libsmb/nmblib.c.
> To make this signal-safe we'd need to change it to
> sys_sendto().
> 
> This is probably ok for dealing with SIGTERM as
> we want to quit in the case anyway. But there
> remain places where we restart the system call
> when interrupted with EINTR and places we don't.
> We should be consistent in this (the sendto
> call above was just one of the places I noticed,
> I haven't had time today to scan for more).

As we don't block SIGUSR1 (used for messaging) and SIGHUP,
blocking SIGTERM makes no sense then. The sento() bug is there
anyway. I'll just push my changes.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 252 bytes
Desc: OpenPGP digital signature
Url : http://lists.samba.org/archive/samba-technical/attachments/20090127/7724e001/signature.bin