Bug 5419 - [review] kill -HUP `pidof spamd` causes the ps name to change from "spamd" to "perl"
[review] kill -HUP `pidof spamd` causes the ps name to change from "spamd" to...
Status: RESOLVED FIXED
Product: Spamassassin
Classification: Unclassified
Component: spamc/spamd
SVN Trunk (Latest Devel Version)
Other other
: P5 minor
: 3.2.2
Assigned To: SpamAssassin Developer Mailing List
:
: 5473 5526 (view as bug list)
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2007-04-17 10:25 UTC by Justin Mason
Modified: 2007-07-04 13:38 UTC (History)
2 users (show)



Attachment Type Modified Status Actions Submitter/CLA Status
fix patch None Justin Mason [HasCLA]

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Mason 2007-04-17 10:25:49 UTC
from bug 5255 comment 2 --

'when you kill -1 spamd, it's ps name 
become perl and then it takes killall -1 perl afterwards to rehash config.'

this is an interesting side effect of how SIGHUP is implemented -- we should
probably try to fix this.
Comment 1 Justin Mason 2007-04-17 10:26:23 UTC
aim at 3.2.0, probably for 3.2.1
Comment 2 Daryl C. W. O'Shea 2007-04-24 15:20:13 UTC
I'm not aware of a way to "un-fix" the effects of bug 5255's r490760.

Can the correct perl interpreter not be specified with PERL_BIN when making the
Makefile, negating the need for r490760?
Comment 3 Justin Mason 2007-05-03 05:00:25 UTC
yeah, I guess the correct fix would be to record the entire ARGV array, and
re-exec using exactly the same ARGV[0] and args, in order to DTRT for these cases:

  perl -T -w ../spamd/spamd.raw
  perl -T -w /usr/bin/spamd
  perl /usr/bin/spamd
  spamd
  /usr/bin/spamd
 
Comment 4 Daryl C. W. O'Shea 2007-05-24 11:34:26 UTC
*** Bug 5473 has been marked as a duplicate of this bug. ***
Comment 5 Sidney Markowitz 2007-06-04 21:40:55 UTC
Moving items off the 3.2.1 queue to 3.2.2 to facilitate a quick release.
If you can get this in Review status right away feel free to move it back
Comment 6 Daryl C. W. O'Shea 2007-06-18 11:48:12 UTC
*** Bug 5526 has been marked as a duplicate of this bug. ***
Comment 7 Justin Mason 2007-06-29 05:04:09 UTC
while waiting for 3.2.2 patch reviews, I decided to have a go at this; I think
this may fix it.

Unfortunately it's not possible to find out exactly what the "real" ARGV array
was when the script was started; instead, what the change does is check the #!
line on the perl script, and if the 2 perl interpreters are the same, it just
execs the script instead of "perl -T -w /the/script".  I think that should work.

btw, this is safer than the alternative approach: 'Can the correct perl
interpreter not be specified with PERL_BIN when making the
Makefile, negating the need for r490760?'

the problem with this is that the user could install using /usr/bin/perl, then
later decide to run spamd using /usr/local/bin/perl (or whatever).  there's
nothing forbidding them from doing this!  when SIGHUP'ed, the spamd may not
restart with the same behaviour since it'd then wind up using a different perl
interpreter. we want to avoid that.

btw -- I have a minor problem trying to repro a test case.  In
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5255#c2 , Martin Lathoud
noted:

'The fix [for bug 5255] creates a side effect at least on Linux, /proc/$PID/exe
points to /usr/bin/perl instead of /usr/bin/spamd, so some proc tools won't find
spamd anymore after a -HUP (i.e pstree, pidof), showing perl instead.'

however when I run spamd, even before SIGHUP, "pidof spamd" only ever lists the
children -- not the parent spamd process!  (That's listed under "pidof perl").

But given that pstree works though, and reports what Martin wanted, I think
that's good enough anyway. let's see how it does in trunk against the buildbots
before I make a patch...

: jm 185...; svn commit -m "bug 5419: when re-execing the perl interpreter in
response to a SIGHUP, the fix for bug 5255 caused the ARGV[0] to change from
'spamd' to 'perl' under certain circumstances.  fix"
Sending        spamd/spamd.raw
Transmitting file data .
Committed revision 551862.
Comment 8 Justin Mason 2007-06-29 05:27:02 UTC
Created attachment 4026 [details]
fix

looks like the buildbots liked it.  here's the patch.
Comment 9 Mark Martinec 2007-06-29 06:56:39 UTC
> Unfortunately it's not possible to find out exactly what the "real"
> ARGV array was when the script was started; ...
> instead, what the change does is check the #!line on the perl script
> [...] btw, this is safer than the alternative approach: 'Can the correct
> perl interpreter not be specified with PERL_BIN when making the Makefile
> the problem with this is that the user could install using /usr/bin/perl,
> then later decide to run spamd using /usr/local/bin/perl (or whatever).

Yes, these are some of the problems associated with restarting a server
with HUP. Without trying to influence a decision here, let me describe
how I handled this in amavisd - a couple of times unsuccessfully bumping
into issues like described here an others, then finally abandoned it
and took another more universal approach (no mountain is so insurmountable
that one could not walk AROUND it).

Additional difficulties (that I had to deal with) with HUP are:
- if a process chrooted itself, it can not escape jail for a restart,
  which would require config file and all Perl modules to be installed
  within a jail, which complicates things and throws away many benefits
  of running chrooted;
- if a process dropped all privileges it can not regain them;
- somehow I could not solve the problem when binding to a specific socket
  was requested and the config changed, a restarted process after a HUP
  just hung (must be a bug somewhere (maybe Net::Server), don't know).

Anyway, for those running amavisd chrooted or when config file was
not accessible after changing UID, the HUP way of restarting was
not possible at all. So instead of having to cater for two types of
usage, I chose to only support one universal way:

When the amavisd command is run with a command-line option 'restart'
(or 'stop'), the program first tries to find existing daemon process
and shut it down, nicely at first (SIGTERM), then with force if it
does not respond after some time. After it has been determined
the old daemon really went away, and a 'restart' (or 'start')
is specified, the interactive process promotes itself to become
a new daemon, just as with a normal initial start.

This approach also works when a currently running daemon is unresponsive.
As a demon is always born out of a shell command (not from another daemon),
its environment, perl interpreter and options is always predictable (explicit),
either during initial start or after a restart.

A possible transition path could be to still support the HUP restarts,
but also provide some restarting and/or shutdown command line option.

Anyway, just food for thought (avoiding potential future problems) ...
Comment 10 Justin Mason 2007-06-29 07:06:36 UTC
(In reply to comment #9)
> A possible transition path could be to still support the HUP restarts,
> but also provide some restarting and/or shutdown command line option.

yeah, that would also work; I agree though that it should be in addition
to this SIGHUP method.
Comment 11 Daryl C. W. O'Shea 2007-07-04 11:50:48 UTC
+1 on 4026
Comment 12 Sidney Markowitz 2007-07-04 13:38:35 UTC
+1, ready to go

Committed to branch 3.2 revision 553325.

Note, this appears to already have been applied to trunk, though I don't notice
that mentioned in a comment here.