343166 - $template->process leaks 512K of RAM per call under mod_perl

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Max Kanat-Alexander]

I noticed that under mod_perl, every time I loaded index.cgi the httpd process became around 512K larger. I did some testing on landfill with GTop, and I've discovered the cause is $template->process. Somewhere it's leaking a lot of memory.

We're using Template-Toolkit v2.15 on landfill.

Comment 1

[Max Kanat-Alexander]

Interesting facts:

If I call $template->process on the same template over and over, the leak is still only 512K.

If I call $template->process on two different templates, the leak becomes about 788K per page-load.

Comment 2

[Max Kanat-Alexander]

Okay, it gets weirder:

If I wrap everything my CGI does inside of a subroutine, we don't leak any memory. Sigh.

Comment 3

[Max Kanat-Alexander]

Okay! Breakthrough!

If I manually delete Bugzilla->request_cache->{template} at the end of a CGI, we have no memory leak.

Sigh.

Unfortunately I'd have to add that code to the end of every CGI, since having an END block inside a module does nothing in mod_perl.

Comment 4

[Bradley Baetz (:bbaetz)]

NOTABUG? It caches.

In startup.pl, you should call the preload function, so that its all precached/compiled/COW/etc. I'll paste code this evening from my install.

Comment 5

[Max Kanat-Alexander]

No, no, it's definitely a bug.

Cal index.cgi over and over, under httpd -X, with all the current mod_perl patches applied.

You'll see it use an *additional* 512K of RAM *every time*. No matter how many times you load the page, it keeps on growing and growing.

I think it's a bug in how Apache2::RequestRec is being destroyed--it's not actually calling delete on each hash key, or something.

Comment 6

[Max Kanat-Alexander]

Perrin Harkins, on the mod_perl list, helped me to figure out that the problem is that Template::Provider's DESTROY method is never getting called, unless I explicitly call "delete" on the hash key.

I suspect this qualifies as a bug in mod_perl, and I've asked the list if they have any ideas on how to work around it.

Comment 7

[Max Kanat-Alexander]

Okay, I can fix this by registering a cleanup handler that manually deletes all of the hash keys in pnotes, at the end of every request.

I'm pretty sure this is still a bug in mod_perl, but we can work around it easily.

Comment 8

[Max Kanat-Alexander]

Attachment: v1

Okay! This works. :-) This fixes the memory leak.

Comment 9

[Dave Miller [:justdave]]

What happens if we make $template process-global instead of per-request?  The whole idea of Template Toolkit under mod_perl is that we don't have to continually re-initialize the Template module on every page load.  When we destroy $template that means we have to create a new one on each page load, which defeats the purpose.  $template ought to be kept as an explicit package-level variable in the Bugzilla object or the Bugzilla::Template object.  i.e. $Bugzilla::template or something.  Does the leak stop if we store it that way instead of in pnotes?  (This would confirm the existance of a bug in pnotes if so)

Comment 10

[Max Kanat-Alexander]

(In reply to comment #9)
> What happens if we make $template process-global instead of per-request?

  I looked around the mod_perl 2 API, and although the API has a way to get a handle for the current process, it's not actually implemented in any version of mod_perl.

  Also, there's no easy way to store a perl data structure somewhere other than a request's pnotes, anyhow.

  We don't lose too much in the way of speed. At least on landfill, I pre-load Bugzilla::Template in the startup file, and so all the loading of the module itself is done before any child starts.

> Does the leak stop if we store it that
> way instead of in pnotes?  (This would confirm the existance of a bug in 
> pnotes if so)

  If we did that, I'd be concerned about threads. Also, since I'm pre-loading various modules, I'd be concerned that the variable would actually become server-global instead of process-global. In general, depending on package variables is probably not a really good idea under mod_perl.

Comment 11

[Max Kanat-Alexander]

Also, another problem is that every time Template::Provider loads a template, it caches it in memory. Eventually, every process would be the size of every single compiled template in Bugzilla, which is too big. Although we could set CACHE_SIZE and see if that helps.

For right now I'd rather go with this, since it stops *all* memory leaks, completely, dead.

If we want to make the $template object per-process, I'd rather do it in another bug.

Comment 12

[Max Kanat-Alexander]

Thanks, justdave! :-)

I do agree that it would be nice to cache the Template object per-process (or per-thread, under a threaded MPM) if possible--I'll look into the possibility.

Checking in Bugzilla.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla.pm,v  <--  Bugzilla.pm
new revision: 1.39; previous revision: 1.38
done