quetz-mod_python-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nicolas Lehuen <nicolas.leh...@gmail.com>
Subject Re: session handling - the next generation
Date Sun, 12 Jun 2005 18:52:05 GMT
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 <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