Edgewall Software

Ticket #166 (closed defect: fixed)

Opened 12 months ago

Last modified 10 months ago

Memory leak in _speedups.c

Reported by: cboos Owned by: cmlenz
Priority: critical Milestone: 0.5
Component: Serialization Version: devel
Keywords: memory Cc:

Description

While tracking down the memory leak we have in Trac 0.11 [t6303], I had a go at removing the _speedups.so library and that effectively seems to be enough to stabilize the memory usage.

Steps to repeat

Use tracd 0.11 with latest Genshi trunk on a t.e.o clone, then repeatedly query: http://localhost:8910/env/timeline?from=2007-08-20T10%3A15%3A47Z%2B0200&precision=second

  • Without _speedups.so, the memory usage stabilizes at 135 Mbytes.
  • With _speedups.so, each request adds between 500 and 800K.

In addition, I'm doing a gc.collect() at the end of each request:

  • trac/web/main.py

     
    382382    finally: 
    383383        if env and not run_once: 
    384384            env.shutdown(threading._get_ident()) 
     385            import gc 
     386            print '----' * 10 
     387            gc.set_debug(gc.DEBUG_UNCOLLECTABLE) 
     388            print 'Garbage Collection', gc.collect() 
     389            print 'Garbage left:', repr(gc.garbage) 
     390            print 'Objects tracked:', repr(len(gc.get_objects())) 
    385391 
    386392def _dispatch_request(req, env, env_error): 
    387393    resp = [] 

The usual output is:

Garbage Collection 0
Garbage left: []
Objects tracked: 47570

(and it stays at 47570)

Reasons

I've not yet located the problem, but so far escape() seems suspicious: the in unicode string doesn't get DECREFed in the inn != 0 case.

Trying the following:

  • genshi/_speedups.c

     
    8787 
    8888    out = (PyUnicodeObject*) PyUnicode_FromUnicode(NULL, len); 
    8989    if (out == NULL) { 
     90        Py_DECREF((PyObject *) in); 
    9091        return NULL; 
    9192    } 
    9293 
     
    130131        inp++; 
    131132    } 
    132133 
     134    Py_DECREF((PyObject *) in); 
     135 
    133136    args = PyTuple_New(1); 
    134137    if (args == NULL) { 
    135138        Py_DECREF((PyObject *) out); 

which seems better for that particular function, doesn't seem to impact significantly on the leak, so there must be other things.

Attachments

cspeedups-fix-memleak-r780.diff (1.4 KB) - added by cboos 12 months ago.
Fix some more issues. The Py_DECREF(unicode); near line 380 seems important. Things seems to be a bit better with that patch, approx 400K increase for each request.
cspeedups-fix-memleak-r780.2.diff (2.2 KB) - added by cboos 12 months ago.
Updated version: this one fixes the leak. The major culprit appeared to be a missing DECREF for the PySequence?_GetItem in Markup_join, near line 269.
cspeedups-memleak-r796.diff (351 bytes) - added by hyuga <hyugaricdeau@…> 10 months ago.
Patch for the missing PyDECREF. Other than that, I didn't notice anything.

Change History

Changed 12 months ago by cboos

Fix some more issues. The Py_DECREF(unicode); near line 380 seems important. Things seems to be a bit better with that patch, approx 400K increase for each request.

Changed 12 months ago by cboos

Updated version: this one fixes the leak. The major culprit appeared to be a missing DECREF for the PySequence?_GetItem in Markup_join, near line 269.

Changed 12 months ago by cmlenz

  • status changed from new to closed
  • resolution set to fixed

Applied in [781]. Thanks!

Changed 10 months ago by hyuga <hyugaricdeau@…>

  • status changed from closed to reopened
  • resolution fixed deleted

Reopening on the suggestion of cboos on the mailing list. The only issue at this point was a missing Py_DECREF(format) at the end of Markup_repr().

Changed 10 months ago by hyuga <hyugaricdeau@…>

Patch for the missing PyDECREF. Other than that, I didn't notice anything.

Changed 10 months ago by cmlenz

  • status changed from reopened to closed
  • resolution set to fixed

Applied in [797]. Thanks!

Add/Change #166 (Memory leak in _speedups.c)

Author



Change Properties
<Author field>
Action
as closed
Next status will be 'reopened'
 
Note: See TracTickets for help on using tickets.