Bug 5626

Summary: [review] "spamassassin" script needs signal handlers
Product: Spamassassin Reporter: Justin Mason <jm>
Component: spamassassinAssignee: SpamAssassin Developer Mailing List <dev>
Status: RESOLVED FIXED    
Severity: minor CC: James.Wilkinson
Priority: P5    
Version: SVN Trunk (Latest Devel Version)   
Target Milestone: 3.2.4   
Hardware: Other   
OS: other   
Whiteboard: ready for commit to 3.2 branch
Bug Depends on:    
Bug Blocks: 5557    
Attachments: fix
fix r2

Description Justin Mason 2007-08-24 09:33:21 UTC
by email regarding bug 5557, Per Kristian Hove noted:

'Hi again. I've attached a backtrace (backtrace.txt) of spamassassin
when this bug [the failure to remove tmp files] occurs.

The backtrace is produced from (and the line numbers relative to)
spamassassin 3.2.3 patched with the attached patch (debug.diff).
Spamassassin was run with TMPDIR set to "./debug".

Installing a signal handler in bin/spamassassin that ignores SIGPIPE
hides/cures the bug.'


backtrace:

[3746] warn: spamassassin: caught SIGPIPE - dumping backtrace.
[3746] warn:  main::backtrace_handler('PIPE') called at
/local/spamassassin-3.2.3/bin/spamassassin line 495
[3746] warn:  eval {...} called at /local/spamassassin-3.2.3/bin/spamassassin
line 495
[3746] warn: 
main::wanted('h','debug/.spamassassin3746r4of60tmp',0,'ARRAY(0x8e5b88)','f')
called at
/local/spamassassin-3.2.3/lib/site_perl/5.8.0/Mail/SpamAssassin/ArchiveIterator.pm
line 346
[3746] warn: 
Mail::SpamAssassin::ArchiveIterator::_run_file('Mail::SpamAssassin::ArchiveIterator=HASH(0x9e8a24)','h','f','debug/.spamassassin3746r4of60tmp',0)
called at
/local/spamassassin-3.2.3/lib/site_perl/5.8.0/Mail/SpamAssassin/ArchiveIterator.pm
line 308
[3746] warn: 
Mail::SpamAssassin::ArchiveIterator::_run_message('Mail::SpamAssassin::ArchiveIterator=HASH(0x9e8a24)','\x{0}\x{0}\x{0}\x{0}hfdebug/.spamassassin3746r4of60tmp')
called at
/local/spamassassin-3.2.3/lib/site_perl/5.8.0/Mail/SpamAssassin/ArchiveIterator.pm
line 292
[3746] warn: 
Mail::SpamAssassin::ArchiveIterator::_run('Mail::SpamAssassin::ArchiveIterator=HASH(0x9e8a24)','ARRAY(0x9e8a3c)')
called at
/local/spamassassin-3.2.3/lib/site_perl/5.8.0/Mail/SpamAssassin/ArchiveIterator.pm
line 285
[3746] warn: 
Mail::SpamAssassin::ArchiveIterator::run('Mail::SpamAssassin::ArchiveIterator=HASH(0x9e8a24)',':detect:debug/.spamassassin3746r4of60tmp')
called at /local/spamassassin-3.2.3/bin/spamassassin line 374
[3746] warn:  eval {...} called at /local/spamassassin-3.2.3/bin/spamassassin
line 374

patch (debug.diff):

+++ bin/spamassassin	Thu Aug 23 15:59:56 2007
@@ -230,6 +230,13 @@
   exit($resphash{'EX_OK'});
 }
 
+sub backtrace_handler {
+  Carp::cluck("spamassassin: caught SIGPIPE - dumping backtrace.\n");
+}
+#$SIG{PIPE} = 'IGNORE';
+$SIG{PIPE} = \&backtrace_handler;
+
+
 # set debug areas, if any specified (only useful for command-line tools)
 if (defined $opt{'debug'}) {
   $opt{'debug'} ||= 'all';



my comments:

it appears that Per is therefore running the "spamassassin" command-line script,
and that script is failing to call ->finish() on a SIGPIPE.  it should catch
SIGPIPEs and clear up appropriately.
Comment 1 Thomas Eisenbarth 2007-08-24 10:07:52 UTC
While you're at it, please be sure to check if the signals HUP, INT, and KILL 
(1, 2, 15) are handled in a similar way. This should handle cleanup in other 
scenarios SpamAssassin users may run into, and maybe get rid of bug 5557, at 
least in the Unix versions.
Comment 2 Justin Mason 2007-09-24 12:35:01 UTC
Created attachment 4129 [details]
fix

here's an implementation of that; it'll clean up the tmpfile on SIGINT, SIGHUP,
SIGTERM and SIGPIPE.
checked into trunk:

: jm 81...; svn commit -m "bug 5626: install a signal handler for SIGHUP,
SIGINT, SIGPIPE and SIGTERM which will remove leftover temp files"
spamassassin.raw
Sending        spamassassin.raw
Transmitting file data .
Committed revision 578928.
Comment 3 Justin Mason 2007-09-24 12:56:58 UTC
actually, that's not quite workable; the info() sub isn't imported.  new fix to
follow.
Comment 4 Justin Mason 2007-09-24 12:57:22 UTC
Created attachment 4131 [details]
fix r2
Comment 5 Daryl C. W. O'Shea 2007-11-06 13:27:31 UTC
+1
Comment 6 Sidney Markowitz 2007-12-21 01:27:18 UTC
+1
Comment 7 Justin Mason 2007-12-21 01:53:33 UTC
thanks guys --applied as r606149.
Comment 8 Thomas Eisenbarth 2008-02-06 06:33:50 UTC
On both Linux and Windows some temp files are still not removed when
spamassassin receives a signal. 
Adding 

  if (defined $tempfile) {
    unlink $tempfile;
    $tempfile = undef;
  }

at the end of kill_handler in the spamassassin script helps, but temp files
created through

my $tmpf = $permsgstatus->create_fulltext_tmpfile($fulltext);

in dccproc_lookup (line 526 in the DCC.pm plugin) will still not be cleaned up
properly.
Any ideas to resolve this?