Hi Jim, After a few checks (unittest + load testing), I've checked in my modifications ; you might want to update and merge it with your code.
Regards, Nicolas 2005/6/12, Jim Gallacher <[EMAIL PROTECTED]>: > Nicolas Lehuen wrote: > > You should cast self to a requestobject* : > > That worked. Running some tests. > > Jim > > > > > ((requestobject*)self)->session > > > > Anyway, I'm currently rewriting the whole request_tp_traverse / > > request_tp_clear / request_tp_clear functions like this : > > > > static void request_tp_dealloc(requestobject *self) > > { > > // de-register the object from the GC > > // before its deallocation, to prevent the > > // GC to run on a partially de-allocated object > > PyObject_GC_UnTrack(self); > > > > request_tp_clear(self); > > > > PyObject_GC_Del(self); > > } > > > > /** > > ** request_tp_traverse > > ** > > * Traversal of the request object > > */ > > static int request_tp_traverse(requestobject* self, visitproc visit, > > void *arg) { > > int result; > > > > if(self->dict) {result = visit(self->dict,arg); if(result) return > > result;} > > if(self->connection) {result = visit(self->connection,arg); > > if(result) return result;} > > if(self->server) {result = visit(self->server,arg); if(result) > > return result;} > > if(self->next) {result = visit(self->next,arg); if(result) return > > result;} > > if(self->prev) {result = visit(self->prev,arg); if(result) return > > result;} > > if(self->main) {result = visit(self->main,arg); if(result) return > > result;} > > if(self->headers_in) {result = visit(self->headers_in,arg); > > if(result) return result;} > > if(self->headers_out) {result = visit(self->headers_out,arg); > > if(result) return result;} > > if(self->err_headers_out) {result = > > visit(self->err_headers_out,arg); if(result) return result;} > > if(self->subprocess_env) {result = > > visit(self->subprocess_env,arg); if(result) return result;} > > if(self->notes) {result = visit(self->notes,arg); if(result) return > > result;} > > if(self->phase) {result = visit(self->phase,arg); if(result) return > > result;} > > > > // no need to Py_DECREF(dict) since the reference is borrowed > > return 0; > > } > > > > static int request_tp_clear(requestobject *self) > > { > > PyObject* tmp; > > tmp=self->dict; self->dict=NULL; Py_XDECREF(tmp); > > tmp=self->connection; self->connection=NULL; Py_XDECREF(tmp); > > tmp=self->server; self->server=NULL; Py_XDECREF(tmp); > > tmp=self->next; self->next=NULL; Py_XDECREF(tmp); > > tmp=self->prev; self->prev=NULL; Py_XDECREF(tmp); > > tmp=self->main; self->main=NULL; Py_XDECREF(tmp); > > tmp=self->headers_in; self->headers_in=NULL; Py_XDECREF(tmp); > > tmp=self->headers_out; self->headers_out=NULL; Py_XDECREF(tmp); > > tmp=self->err_headers_out; self->err_headers_out=NULL; Py_XDECREF(tmp); > > tmp=self->subprocess_env; self->subprocess_env=NULL; Py_XDECREF(tmp); > > tmp=self->notes; self->notes=NULL; Py_XDECREF(tmp); > > tmp=self->phase; self->phase=NULL; Py_XDECREF(tmp); > > tmp=self->hlo; self->hlo=NULL; Py_XDECREF(tmp); > > tmp=self->dict; self->dict=NULL; Py_XDECREF(tmp); > > tmp=self->dict; self->dict=NULL; Py_XDECREF(tmp); > > tmp=self->dict; self->dict=NULL; Py_XDECREF(tmp); > > tmp=self->dict; self->dict=NULL; Py_XDECREF(tmp); > > > > return 0; > > } > > > > As you can guess from the source code, we'll have to add some code for > > the session member when you'll integrate your code in subversion. > > > > Regards, > > Nicolas > > > > 2005/6/12, Jim Gallacher <[EMAIL PROTECTED]>: > > > >>Nicolas, > >> > >>It fails to compile when I add your bit of code. Sure looks like it > >>should work though. > >> > >>requestobject.c: In function `request_tp_traverse': > >>requestobject.c:1539: error: structure has no member named `session' > >>requestobject.c:1540: error: structure has no member named `session' > >> > >>Just to be clear, it compiles and runs just fine without the extra check > >> added into request_tp_traverse. > >> > >>Nicolas Lehuen wrote: > >> > >>>I'm re-reading the comment I wrote when I implemented tp_traverse : > >>> > >>>// only traverse its dictionary since other fields defined in > >>>request_rec_mbrs with type T_OBJECT > >>>// cannot be the source of memory leaks (unless you really want it) > >>> > >>>Turns out that we should at least traverse the next and prev in case > >>>someone does something like req.next.my_prev = req. That's what I > >>>meant by "unless you really want it". > >> > >>I wondered about that. :) > >> > >>Jim > >> > >> > >>>It's pretty difficult to get > >>>there (you need an internal redirect), but you can. So I'm going to > >>>have a look at re-implementing the tp_traverse handler. > >>>Regards, > >>>Nicolas > >>> > >>>2005/6/12, Nicolas Lehuen <[EMAIL PROTECTED]>: > >>> > >>> > >>>>Duh, I get it. If you add a member to the request object, and this > >>>>member is not referenced in the request object's dictionary, then you > >>>>have to add a special case for it in the tp_traverse handler. In > >>>>requestobject.c : > >>>> > >>>>/** > >>>>** request_tp_traverse > >>>>** > >>>>* Traversal of the request object > >>>>*/ > >>>>static int request_tp_traverse(PyObject *self, visitproc visit, void > >>>>*arg) { > >>>> PyObject *dict,*values,*item,*str; > >>>> int i,size; > >>>> > >>>> // only traverse its dictionary since other fields defined in > >>>>request_rec_mbrs with type T_OBJECT > >>>> // cannot be the source of memory leaks (unless you really want it) > >>>> dict=*_PyObject_GetDictPtr(self); > >>>> if(dict) { > >>>> // this check is not needed, I guess, _PyObject_GetDictPtr > >>>>always give a pointer to a dict object. > >>>> if(PyDict_Check(dict)) { > >>>> i = visit(dict,arg); > >>>> if(i) { > >>>> ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, > >>>>((requestobject*)self)->request_rec, "%s:%i Call to visit() > >>>>failed",__LINE__,__FILE__); > >>>> // no need to Py_DECREF(dict) since the reference is > >>>> borrowed > >>>> return i; > >>>> } > >>>> } > >>>> else { > >>>> ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, > >>>>((requestobject*)self)->request_rec, "%s:%i Expected a > >>>>dictionary",__LINE__,__FILE__); > >>>> } > >>>> } > >>>> else { > >>>> ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, > >>>>((requestobject*)self)->request_rec, "%s:%i Expected a > >>>>dictionary",__LINE__,__FILE__); > >>>> } > >>>> > >>>> /* This is the new code that needs to be added to support the new > >>>>session member */ > >>>> if(self->session) { > >>>> i = visit(self->session,arg); > >>>> if(i) { > >>>> ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, > >>>>((requestobject*)self)->request_rec, "%s:%i Call to visit() > >>>>failed",__LINE__,__FILE__); > >>>> return i; > >>>> } > >>>> } > >>>> > >>>> // no need to Py_DECREF(dict) since the reference is borrowed > >>>> return 0; > >>>>} > >>>> > >>>>Regards, > >>>>Nicolas > >>>> > >>>>2005/6/12, Jim Gallacher <[EMAIL PROTECTED]>: > >>>> > >>>> > >>>>>Hi Nicolas, > >>>>> > >>>>>Nicolas Lehuen wrote: > >>>>> > >>>>> > >>>>>>Hi Jim, > >>>>>> > >>>>>>How where you creating the session object in requestobject.c ? If you > >>>>>>were using PythonObject_New, then this may explain the memory leak. > >>>>>>Objects that must be managed by the garbage collector have to be > >>>>>>created with PyObject_GC_New. > >>>>> > >>>>>The c-code loads the python module and calls a function which generates > >>>>>the session object. > >>>>> > >>>>>src/requestobject.c > >>>>>static PyObject *req_get_session(requestobject *self, PyObject *args) > >>>>>{ > >>>>> PyObject *m; // session module > >>>>> PyObject *sid; // session id > >>>>> PyObject *result; > >>>>> > >>>>> if (!self->session) { > >>>>> sid = PyObject_CallMethod(self->subprocess_env, "get", > >>>>> "(ss)","REDIRECT_PYSID", ""); > >>>>> > >>>>> /******** > >>>>> * This is where the session instance is created > >>>>> ********/ > >>>>> m = PyImport_ImportModule("mod_python.session2.TestSession"); > >>>>> self->session = PyObject_CallMethod(m, "create_session", "(OO)", > >>>>> self, sid); > >>>>> > >>>>> Py_DECREF(m); > >>>>> Py_DECREF(sid); > >>>>> if (self->session == NULL) > >>>>> return NULL; > >>>>> } > >>>>> result = self->session; > >>>>> Py_INCREF(result); > >>>>> return result; > >>>>>} > >>>>> > >>>>>---------------------------------------------------------------- > >>>>>mod_python/session2/TestSession.py > >>>>># emulate a simple test session > >>>>>import _apache > >>>> > >>>>>from mod_python.Session import _new_sid > >>>> > >>>>>class TestSession(object): > >>>>> > >>>>> def __init__(self, req, sid=0, secret=None, timeout=0, lock=1): > >>>>> req.log_error("TestSession.__init__") > >>>>> # Circular Reference causes problem > >>>>> self._req = req > >>>>> > >>>>> # keeping a reference to the server object does > >>>>> # NOT cause a problem > >>>>> self._server = req.server > >>>>> self._lock = 1 > >>>>> self._locked = 0 > >>>>> self._sid = _new_sid(req) > >>>>> self.lock() > >>>>> self.unlock() > >>>>> > >>>>> def lock(self): > >>>>> if self._lock: > >>>>> _apache._global_lock(self._req.server, self._sid) > >>>>> self._locked = 1 > >>>>> > >>>>> def unlock(self): > >>>>> # unlock will ocassionally segfault > >>>>> if self._lock and self._locked: > >>>>> _apache._global_unlock(self._req.server, self._sid) > >>>>> self._locked = 0 > >>>>> > >>>>>def create_session(req,sid): > >>>>> return TestSession(req,sid) > >>>>> > >>>> > >> > > > >