You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@bugzilla.spamassassin.org on 2005/07/18 12:24:19 UTC

[Bug 4488] New: Process size growing with each processed message

http://bugzilla.spamassassin.org/show_bug.cgi?id=4488

           Summary: Process size growing with each processed message
           Product: Spamassassin
           Version: 3.0.4
          Platform: All
        OS/Version: other
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Libraries
        AssignedTo: dev@spamassassin.apache.org
        ReportedBy: ifettich@netsoft.ro


Using SpamAssassin's Libraries to deal with many messages makes SA process grow
and grow and grow...

I'll add an short script that reproduces the problem; looks like memory is not
freed correctly, altough 'finish' is called after each message.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4488





------- Additional Comments From jm@jmason.org  2005-07-18 09:51 -------
sounds like $sa->finish() isn't clearing up some circular ref.

could you attach the full script where the problem has disappeared?  for example
I suspect you also moved $sa->finish() outside the loop, too.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] [review] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4488


felicity@apache.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #3549 is|0                           |1
           obsolete|                            |




------- Additional Comments From felicity@apache.org  2006-06-27 04:34 -------
Created an attachment (id=3561)
 --> (http://issues.apache.org/SpamAssassin/attachment.cgi?id=3561&action=view)
suggested patch

I already committed this to 3.2 btw:

Sending        lib/Mail/SpamAssassin/Bayes.pm
Sending        lib/Mail/SpamAssassin.pm
Transmitting file data ..
Committed revision 417337.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4488


ifoutch@parusinteractive.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ifoutch@parusinteractive.com






------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4488





------- Additional Comments From ifettich@netsoft.ro  2005-07-18 10:28 -------
Created an attachment (id=3030)
 --> (http://bugzilla.spamassassin.org/attachment.cgi?id=3030&action=view)
script demonstrating a work-around for the problem.

I attached the 'corrected' script also, which behaves as expected. 

Yes, I think it has to do with sa_finish().

I tried to see if I can go into it further to see if I could offer a patch, but
I'm not aquainted yet enough with SA's code for that.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4488





------- Additional Comments From dallase@nmgi.com  2006-06-05 14:00 -------
we recently covered this same thing here
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4897

there is no reason to M::SA->new() each time inside the loop.  one instance
outside the loop is sufficient.  if you want to renew the instance once in a
while (ie --max-conn-per-child in spamd), then you could do that inside the loop
by tracking iteration count.

FWIW, 3.0.4 and above would be effected by 
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4859

have you tested with a current SVN checkout?






------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4488





------- Additional Comments From bryanh@giraffe-data.com  2006-06-10 21:32 -------
Created an attachment (id=3549)
 --> (http://issues.apache.org/SpamAssassin/attachment.cgi?id=3549&action=view)
Add a finish() to fix a memory leak

I believe I found the source of a memory leak bug in 3.1.1:		       

									       

Mail::SpamAssassin::BayesStore::DBM contains a circular reference to a	       

Mail::SpamAssassin::Bayes, and has no finish() to break it.  The Bayes lives   

forever.								       

									       

The attached patch looks like it would fix it.			       
									       

There's no telling if this is what caused the leak originally reported (there's

evidence to the contrary), but a very simple loop like that reported, only     

simpler because it uses an empty configuration, has no leak in 3.1.1 after I   

apply this.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] [review] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4488


jm@jmason.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|needs 2 votes               |needs 1 vote




------- Additional Comments From jm@jmason.org  2006-06-29 09:25 -------
+1

I wonder why the memory-leak detection test case in our test suite didn't catch
that...



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4488





------- Additional Comments From bryanh@giraffe-data.com  2006-06-05 07:57 -------
OK, never mind.  I confused the two finish()'s in losif's test, and didn't 
realize Mail::Spamassin needed one.  That explains much of the leaking in my 
tests.

I'll keep looking.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4488





------- Additional Comments From ifettich@netsoft.ro  2005-07-18 03:41 -------
Plugins don't play any role. 

I commented out all 'loadplugin's from init.pre, and the problem remains. 



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4488





------- Additional Comments From bryanh@giraffe-data.com  2006-06-05 06:25 -------
(In reply to comment #4)
> sounds like $sa->finish() isn't clearing up some circular ref.

Apparently, $sa->finish isn't even required here, since the per-message
expansion doesn't happen when the per-message $sa->finish is removed entirely.
Rather, it looks like a problem with Mail::SpamAssassin::new.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4488


felicity@apache.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|Undefined                   |3.1.4






------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4488





------- Additional Comments From bryanh@giraffe-data.com  2006-06-05 06:39 -------
I've simplified the demonstration of the problem further.  Here's a loop of 
nothing but Mail::SpamAssassin::new, with one rule, and it eats 42K per 
iteration.

use Mail::SpamAssassin;

foreach my $i (1..10) {
    my $spamAssassin = new Mail::SpamAssassin({config_text => ''});

    printProcessMemorySize();
}

I will proceed to whittle down Mail::SpamAssassin::new() and see if I can figure 
out what memory is not getting returned (circular reference, maybe).  If anyone 
knows a better way to track a Perl/SA memory leak, let me know (or just do it 
yourself before I waste too much time!).




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4488





------- Additional Comments From ifettich@netsoft.ro  2005-07-18 03:30 -------
Created an attachment (id=3028)
 --> (http://bugzilla.spamassassin.org/attachment.cgi?id=3028&action=view)
short perl script to reveal the problem

I'm not sure yet if it's caused by the main SA code or by the plugins being
mentioned in init.pre. 

However, I think what I see (and the script shows) should not happen.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] [review] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4488


spamassassin@dostech.ca changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|needs 1 vote                |can be committed




------- Additional Comments From spamassassin@dostech.ca  2006-07-09 22:06 -------
+1



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] [review] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4488


felicity@apache.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED




------- Additional Comments From felicity@apache.org  2006-07-10 00:20 -------
Sending        lib/Mail/SpamAssassin/Bayes.pm
Sending        lib/Mail/SpamAssassin.pm
Transmitting file data ..
Committed revision 420378.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://bugzilla.spamassassin.org/show_bug.cgi?id=4488





------- Additional Comments From ifettich@netsoft.ro  2005-07-18 03:54 -------
Taking the lines 

  my $sa = Mail::SpamAssassin->new();
     $sa->compile_now(0, 0);

outside the loop make the problem 'disappear'. 




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4488





------- Additional Comments From bryanh@giraffe-data.com  2006-06-10 21:17 -------
(In reply to comment #9)
> there is no reason to M::SA->new() each time inside the loop.  one instance
> outside the loop is sufficient.

Unless of course you're trying to demonstrate a leak in create/destroy of
M::SA, which is what Losef and I were trying to do.
 
> have you tested with a current SVN checkout?

No, I am using 3.1.1.  The history of this bugzilla bug made it look like memory 
leaks live a long time in SpamAssassin, so going even newer didn't seem like the 
most productive use of time.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 4488] [review] Process size growing with each processed message

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4488


felicity@apache.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Process size growing with   |[review] Process size
                   |each processed message      |growing with each processed
                   |                            |message
  Status Whiteboard|                            |needs 2 votes




------- Additional Comments From felicity@apache.org  2006-06-27 04:33 -------
Hrm.  After spending a good bit thinking about this and going through the code,
I've decided that this patch does not fully addresses the issue, and ends up
causing some other issues.  Specifically, the problem is that
M::SA::finish_learner() (which calls M::SA::Bayes::finish()) is called
specifically to only untie the DB, but the rest of the object should be left
intact.  Only when M::SA::finish() is called should the Bayes and BayesStore
objects be wripped down.

So my thought (patch attached shortly) is for finish_learner() to call
Bayes::sanity_check_is_untied() (with a slight API addition) which will just
untie if we're currently tied. finish() will call Bayes::finish() which will go
ahead and untie as well as drop the reference to BayesStore.  That will then let
BayesStore get cleaned up, and finish() then drops the Bayes reference and Bayes
gets cleaned up, and things are fine.

Please give this patch a try. :)



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.