Asp Forum - FCGI 0.8.5 -- patch for major memory leaks

Kirk Haines

3/22/2005 5:08:00 PM

Nutshell: The 0.8.5 version of the C extension for FCGI for Ruby leaks,
badly. I have a patch file at http://enigo.com/downloads/fc... if
you want to skip the explanation below and go right to it.

-----

http://enigo.com/projects/iowa/fcgi...

When using the 0.8.5 version of the Ruby FCGI module with the C extension
instead of the pure Ruby extension, I noticed a substantial memory leak, to
the tune of around 16k+/request. A little poking through the code turned up
two seperate regions where memory is allocated without any provision to
free it later.

The first such instance occurs in the

fcgi_s_accept()

function. Memory is allocated to hold the FCGI request structure
(FCGX_Request), with this line:

req = ALLOC(FCGX_Request);

Then a data struct (fcgi_data), which has slots to hold the FCGI request,
the data streams, and the environment variables, is turned into a Ruby
object with a call to

Data_Make_Struct

A mark function is provided that marks the stream and environment slots for
the garbage collector. The request slot, however, is not marked since it
points to a C struct and not to a Ruby object. However, a free function is
not provided to cleanup either the root object or the request structure
that it contains. This is the first memory leak. It is fixed by providing a
function that will free this memory. This bug, with the same fix, was also
mentioned on ruby-core
(http://www.ruby-talk.org/cgi-bin/scat.rb/ruby/ruby...)last year in
reference to version 0.8.4.

Add this:

static void fcgi_free_req(fcgi_data *data)
{
free(data-<req);
free(data);
}

And then replace this:

obj = Data_Make_Struct(self, fcgi_data, fcgi_mark, 0, data);

with this:

obj = Data_Make_Struct(self, fcgi_data, fcgi_mark, fcgi_free_req, data);

The second memory leak is found in the

fcgi_stream_read()

function. It allocates a buffer to use when reading data from the stream,
but that buffer is not freed at any of the points where the function can be
exited. The fix is just to add

free(buff);

before each of the possible exit points.

With these two sets of changes in place, I have now ran more than two
million requests through FCGI, with a completely flat memory utilization. A
patch file with these changes can be found at

http://enigo.com/downloads/fc...

Thanks,

Kirk Haines

4 Answers

Tobias Luetke

3/22/2005 9:12:00 PM

This is a major fix for the web crowd.

Can we get a new fcgi release pretty please?

Thanks a lot for this Kirk

On Wed, 23 Mar 2005 02:09:55 +0900, Kirk Haines <wyhaines@gmail.com> wrote:
> Nutshell: The 0.8.5 version of the C extension for FCGI for Ruby leaks,
> badly. I have a patch file at http://enigo.com/downloads/fc... if
> you want to skip the explanation below and go right to it.
>
> -----
>
> http://enigo.com/projects/iowa/fcgi...
>
> When using the 0.8.5 version of the Ruby FCGI module with the C extension
> instead of the pure Ruby extension, I noticed a substantial memory leak, to
> the tune of around 16k+/request. A little poking through the code turned up
> two seperate regions where memory is allocated without any provision to
> free it later.
>
> The first such instance occurs in the
>
> fcgi_s_accept()
>
> function. Memory is allocated to hold the FCGI request structure
> (FCGX_Request), with this line:
>
> req = ALLOC(FCGX_Request);
>
> Then a data struct (fcgi_data), which has slots to hold the FCGI request,
> the data streams, and the environment variables, is turned into a Ruby
> object with a call to
>
> Data_Make_Struct
>
> A mark function is provided that marks the stream and environment slots for
> the garbage collector. The request slot, however, is not marked since it
> points to a C struct and not to a Ruby object. However, a free function is
> not provided to cleanup either the root object or the request structure
> that it contains. This is the first memory leak. It is fixed by providing a
> function that will free this memory. This bug, with the same fix, was also
> mentioned on ruby-core
> (http://www.ruby-talk.org/cgi-bin/scat.rb/ruby/ruby...)last year in
> reference to version 0.8.4.
>
> Add this:
>
> static void fcgi_free_req(fcgi_data *data)
> {
> free(data-<req);
> free(data);
> }
>
> And then replace this:
>
> obj = Data_Make_Struct(self, fcgi_data, fcgi_mark, 0, data);
>
> with this:
>
> obj = Data_Make_Struct(self, fcgi_data, fcgi_mark, fcgi_free_req, data);
>
> The second memory leak is found in the
>
> fcgi_stream_read()
>
> function. It allocates a buffer to use when reading data from the stream,
> but that buffer is not freed at any of the points where the function can be
> exited. The fix is just to add
>
> free(buff);
>
> before each of the possible exit points.
>
> With these two sets of changes in place, I have now ran more than two
> million requests through FCGI, with a completely flat memory utilization. A
> patch file with these changes can be found at
>
> http://enigo.com/downloads/fc...
>
> Thanks,
>
> Kirk Haines
>
>

--
Tobi
http://www.sn... - Snowboards that don't suck
http://www.h... - Open source book authoring
http://blog.le... - Technical weblog

zhekov

3/23/2005 1:57:00 AM

> This is a major fix for the web crowd.
>
> Can we get a new fcgi release pretty please?
>

For unpatient Gentoo lovers:
[ http://bgjap.net/code/ebuilds/ruby-fcgi-0.8.5-r1.ebui... ]

Matt Mower

3/23/2005 8:08:00 AM

On Wed, 23 Mar 2005 02:09:55 +0900, Kirk Haines <wyhaines@gmail.com> wrote:
> Nutshell: The 0.8.5 version of the C extension for FCGI for Ruby leaks,
> badly. I have a patch file at http://enigo.com/downloads/fc... if
> you want to skip the explanation below and go right to it.
> [...]
> With these two sets of changes in place, I have now ran more than two
> million requests through FCGI, with a completely flat memory utilization. A
> patch file with these changes can be found at
>

Nicely done.

Matt

--
Matt Mower :: http://matt...

George Moschovitis

3/23/2005 8:41:00 AM

Thanks a lot!

tml (George Moschovitis)

--
http://nitro.rub...

comp.lang.ruby

FCGI 0.8.5 -- patch for major memory leaks

Kirk Haines

Tobias Luetke

zhekov

Matt Mower

George Moschovitis

x Login to ForumsZone