Bug 4631 - [review] __DIE__ hooks affecting exception handling by evals
Summary: [review] __DIE__ hooks affecting exception handling by evals
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Libraries (show other bugs)
Version: 3.1.0
Hardware: Other other
: P5 normal
Target Milestone: 3.1.1
Assignee: Daryl C. W. O'Shea
URL:
Whiteboard: ready to commit
Keywords:
: 4659 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-10-11 10:00 UTC by Justin Mason
Modified: 2005-12-08 18:25 UTC (History)
3 users (show)


Attachment Type Modified Status Actions Submitter/CLA Status
patch patch None Daryl C. W. O'Shea [HasCLA]
additional patch patch None Justin Mason [HasCLA]
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Mason 2005-10-11 10:00:26 UTC 
debian bug 333330 notes:

starting spamassassin via the init script resolves with the error:

mybabe:~# /etc/init.d/spamassassin restart
Restarting SpamAssassin Mail Filter Daemon: [9296] error: Can't locate
object method "new" via package "Ne
t::DNS::Resolver" at /usr/share/perl5/Mail/SpamAssassin/DnsResolver.pm
line 87.
spamd.



looks to me a lot like the reporter doesn't have Net::DNS installed, but
DnsResolver is using that module without eval{} protection -- hence causing a
crash.   Net::DNS is still listed as an optional module in our documentation.
Comment 1 Justin Mason 2005-10-11 10:00:56 UTC 
3.1.1.
Comment 2 Duncan Findlay 2005-11-04 21:55:09 UTC 
Turns out this error message and similar ones happen when Net::DNS is not
installed. Surely there must be a way of silencing these! (I hope...)

The most common error message is:
Restarting SpamAssassin Mail Filter Daemon: [13804] error: Can't locate
Net/DNS.pm in @INC (@INC contains: ../lib /usr/share/perl5 /etc/perl
/usr/local/lib/perl/5.8.7 /usr/local/share/perl/5.8.7 /usr/lib/perl5
/usr/lib/perl/5.8 /usr/share/perl/5.8 /usr/local/lib/site_perl
/usr/local/lib/perl/5.8.4 /usr/local/share/perl/5.8.4 /usr/local/lib/perl/5.8.0
/usr/local/share/perl/5.8.0) at
/usr/share/perl5/Mail/SpamAssassin/DnsResolver.pm line 86.
spamd.

Note that these messages are just warnings, spamd still starts; but they are
logged as "error".
Comment 3 Duncan Findlay 2005-11-04 21:55:29 UTC 
Net::DNS and its dependencies I should say
Comment 4 Daryl C. W. O'Shea 2005-11-12 01:26:26 UTC 
Created attachment 3247 [details]
patch

Stupid broken __DIE__ hooks... evals often don't work as exception handlers
when you've installed your own __DIE__ hooks.

Getting Logger.pm to check to see if the message is coming from an eval in one
of our modules should do the trick.
Comment 5 Daryl C. W. O'Shea 2005-11-12 01:29:10 UTC 
trunk r332683
Comment 6 Daryl C. W. O'Shea 2005-11-12 01:31:04 UTC 
*** Bug 4659 has been marked as a duplicate of this bug. ***
Comment 7 Justin Mason 2005-11-12 02:22:40 UTC 
+1
Comment 8 Justin Mason 2005-11-28 21:51:28 UTC 
'evals often don't work as exception handlers when you've installed your own
__DIE__ hooks'

well, they do work -- it's just that the fake-die messages used (kludgily) as
exceptions in perl are "leaked".  btw, I knew there was a good way to fix this;
here it is, as described in perldoc -f eval:

When using the eval{} form as an exception trap in libraries, you may wish not
to trigger any __DIE__ hooks that user code may have installed. You can use the
local $SIG{__DIE__} construct for this purpose, as shown in this example:

    # a very private exception trap for divide-by-zero
    eval { local $SIG{'__DIE__'}; $answer = $a / $b; };
    warn $@ if $@;

see http://www.perl.com/doc/manual/html/pod/perlfunc/eval.html .

r349500 is a checkin that adds those, as well, in trunk, in addition to patch 3247.

ps: we really need a way to consolidate those timeout blocks; way too much code
duplication there...
Comment 9 Justin Mason 2005-11-28 21:55:33 UTC 
Created attachment 3281 [details]
additional patch

here's a patch that adds the local{__DIE__} stuff.
Comment 10 Daryl C. W. O'Shea 2005-11-28 22:14:57 UTC 
Yeah, I know they work, but not ideally.  I actually thought of that paragraph
from eval's perldoc when I first saw the problem and considered it as a solution.

However, I figured the chances of everyone remembering to do that in both our
code AND third party-plugins were pretty slim.

Using the local $SIG constructs are certainly faster, so if people remember they
might at well use them.  The other code can catch leaks from plugins, etc.
Comment 11 Daryl C. W. O'Shea 2005-11-28 22:23:10 UTC 
BTW, +1 on adding 3281, so 1 vote needed for both patches for 3.1.
Comment 12 Justin Mason 2005-11-28 22:35:33 UTC 
yeah, this is not a replacement for patch 3247; purely supplementary, for
efficiency.

I have an idea on how to consolidate those timeouts using closures; separate
issue though.
Comment 13 Justin Mason 2005-12-08 22:18:03 UTC 
ping: 1 more vote needed
Comment 14 Duncan Findlay 2005-12-09 03:20:51 UTC 
+1
Comment 15 Daryl C. W. O'Shea 2005-12-09 03:25:14 UTC 
3.1: Committed revision 355320.