quetz-mod_python-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Gallacher <jg.li...@sympatico.ca>
Subject Re: session handling - the next generation
Date Sun, 12 Jun 2005 20:57:59 GMT
Nicolas Lehuen wrote:
> 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.

I'm still getting a memory leak with the merged code. Should I commit my 
changes anyway, or maybe we could create new svn branch for this testing?

Jim

> Regards,
> Nicolas
> 
> 2005/6/12, Jim Gallacher <jg.lists@sympatico.ca>:
> 
>>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 <jg.lists@sympatico.ca>:
>>>
>>>
>>>>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 <nicolas.lehuen@gmail.com>:
>>>>>
>>>>>
>>>>>
>>>>>>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 <jg.lists@sympatico.ca>:
>>>>>>
>>>>>>
>>>>>>
>>>>>>>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)
>>>>>>>
>>>>>>
>>
> 


Mime
View raw message