Bug 4218 - PerMsgStatus $status->finish() not getting registered
Summary: PerMsgStatus $status->finish() not getting registered
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Rules (Eval Tests) (show other bugs)
Version: 3.0.2
Hardware: All Linux
: P3 minor
Target Milestone: 3.1.0
Assignee: Justin Mason
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-03-22 10:56 UTC by Eric A. Hall
Modified: 2005-04-11 11:59 UTC (History)
0 users


Attachment Type Modified Status Actions Submitter/CLA Status
fix 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 Eric A. Hall 2005-03-22 10:56:18 UTC 
http://spamassassin.apache.org/full/3.0.x/dist/doc/Mail_SpamAssassin_PerMsgStatus.html
says that $status->finish() should get called when the PerMsgStatus object is
destroyed. However, the finish() function in my plug-in is not getting
registered or called.
Comment 1 Daniel Quinlan 2005-04-06 00:34:23 UTC 
The plugin function name needs to be per_msg_finish().
Comment 2 Eric A. Hall 2005-04-06 05:48:54 UTC 
doesn't work for me. perhaps there is a version comment that would be
appropriate. perhaps the docs are wrong, and the bug needs to be changed.

?
Comment 3 Justin Mason 2005-04-11 19:57:39 UTC 
Created attachment 2783 [details]
fix 

argh, this looked easy and then wasn't ;)

sure enough, there was a bug.  Basically, a temporary PerMsgStatus object is
created *very* early on -- in lib/Mail/SpamAssassin/Message/Metadata/Received
-- before the plugin was loaded, resulting in the 'per_msg_finish' callbacks
being cached without the plugin's method in the list.

fix was to get that object created *once* in Mail::SpamAssassin -- not every
time a message is parsed.  (it's much more efficient this way anyway.)

long-term fix would be to get some of the DNS-lookup code out of Dns.pm, since
having it there means it unnecessarily relies on a PerMsgStatus object being in
existence... which is really not necessary for lookups in the Received header
parsing code.

couple of other minor fixes: added some doco to Plugin::per_msg_finish() to
note that only circular refs need to be cleaned up normally. and added a test
to the test suite.
Comment 4 Justin Mason 2005-04-11 19:59:51 UTC 
applied; r161011