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)





Reply via email to