Re: session handling - the next generation
Nicolas Lehuen wrote:
Hi Jim,
Have you added the corresponding code in request_tp_clear and
request_tp_traverse ?
Yes.
I think you can check-in your code, since the debate about import
looks like it is going to take som time before the next release...
Will do.
Jim
Regards,
Nicolas
2005/6/12, Jim Gallacher [EMAIL PROTECTED]:
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 [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
Re: session handling - the next generation
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)
Re: session handling - the next generation
You should cast self to a requestobject* :
((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 =
Re: session handling - the next generation
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);
Re: session handling - the next generation
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;
