<div dir="ltr">I agree. When you know you are heading for a prompt exit from a signal, don't try to clean up anything, especially the alloc arena. Log a message fast, and then exit.<div><br></div><div>It is my own painful experience that when terminating signal is received, especially one generated by the OS or the kernel, one often can no longer trust the coherency of alloc, of stdio, of the in-process DNS/name cache. Thus, don't try to clean anything up, don't try to look anything up, and hope that you can get one more write to stderr and/or syslog, and just exit.</div><div><br></div><div>Also, there are few things as rage inducing to a sysadmin or to a devops developer as a process that thinks it knows better than you do when it receives a signal, or that now is a good time to spend time doing anything other than exiting.</div><div><br></div><div>The sysadmin or devops developer will forgive all the alloc leak warnings.<br><div><br></div><div>POSIX guarantees that process memory is reclaimed, and that all fd's get closed. Use that guarantee.</div></div><div><br></div><div>If the day comes that we run on an OS that fails that guarantee, instead of trying to work around it, instead we will file bugs against that OS, and not mess up NTPsec to cover for their mistake.</div><div><br></div><div>..m</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 30, 2015 at 5:39 AM Eric S. Raymond <<a href="mailto:esr@thyrsus.com">esr@thyrsus.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hal Murray <<a href="mailto:hmurray@megapathdsl.net" target="_blank">hmurray@megapathdsl.net</a>>:<br>
><br>
> I think it only gets tested on NetBSD.<br>
><br>
> The first problem is that it doesn't include the header file.<br>
> The second is that mdns is over in ntpd.c<br>
><br>
> I think the idea of moving that much actual code over there is a bad idea.<br>
<br>
I reverted that change last night. You're right, it was a bad idea.<br>
<br>
> This looks fishy :<br>
> sig_desc = NULL;<br>
> sig_desc = strsignal(sig);<br>
> if (sig_desc == NULL)<br>
> sig_desc = "";<br>
><br>
> It's leftover from when strsignal might not exist and didn't get cleaned up<br>
> when you removed that ifdef test. My man page says some systems (not Linux)<br>
> might return NULL on an invalid signal. I assume anything passed to that<br>
> routine will be valid.<br>
<br>
Yes, I guess the "sig_desc = NULL;" can go<br>
<br>
> There are 2 ways into that code. One is via a kill command, probably from<br>
> something like "service ntpd stop". The other is from a SIGBUS. The SIGBUS<br>
> case should probably bypass the cleanup and exit non-zero.<br>
><br>
> I think the idea of doing more than a simple log message and exit is to free<br>
> up all the mallocs so the tool that monitors malloc usage would see a close<br>
> to clean result at exit and everything left would be lost memory. I don't<br>
> see the code I expect. There is a lot of mallocing in the config file parser<br>
> and big chunks of memory for the mrulist. The config stuff is using an<br>
> atexit-hook. That's probably a bad idea if we are going to catch SIGBUS and<br>
> expect to do anything.<br>
<br>
You're almost certainly right about all this. Feel free to clean up<br>
as you like; I can't get very excited about it, as I judge the<br>
objective (free up all mallocs before exit) to be unachievable in<br>
practice given NTP's level of allocation complexity and multiple exit<br>
paths.<br>
--<br>
<a href="<a href="http://www.catb.org/~esr/" rel="noreferrer" target="_blank">http://www.catb.org/~esr/</a>">Eric S. Raymond</a><br>
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@ntpsec.org" target="_blank">devel@ntpsec.org</a><br>
<a href="http://lists.ntpsec.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ntpsec.org/mailman/listinfo/devel</a><br>
</blockquote></div>