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)
> >>>>>
> >>>>
> >>
> >
>
>
