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

Reply via email to