[mod_python] This is just friggin' weird...

Graham Dumpleton wrote:
> 
> On 24/06/2005, at 9:14 AM, Jim Gallacher wrote:
> 
>>> Are you accessing the "session" object in the error page?
>>> I have no access to mod_python source code right now to check,
>>> but vaguely remember that if error page tries to access "session"
>>> object it will deadlock. This is part of the reasons for having the
>>> PSP object check if req.session exists and use it instead of creating
>>> a new one each time, as proposed in one of the JIRA bug reports
>>> I have posted. I'll post the JIRA link later.
>>
>>
>> This is solved with the req.get_session() code that I've checked
>> into svn trunk. See http://issues.apache.org/jira/browse/MODPYTHON-59
> 
> 
> I can't refer to the JIRA bug report at the moment as JIRA is broken,
> but you might want to look at some of my comments if you can find the
> bug report related to psp and form objects. It talks about the session
> object in there as well and one of the issues I pointed out is whether
> when the PSP object doesn't create the session in the first place,
> should it assume that it should save the session. With the way you
> have made the change, the PSP object will save the session even if
> it didn't create it in the first place. Am not sure if this is the
> appropriate thing to do or not.
> 

I'm familiar with the issue you mention. The only change I've made in 
the PSP class is to replace "session = Session.Session()" with "session 
= req.get_session()". Other than that the behaviour is identical to 3.1.4.

So the question is really whether the current behaviour of psp pages 
should change. I totally get what you are saying about auto-saving. The 
problem is if we change it now we'll likely break existing code. I can 
see the "Hey, I upgraded to 3.2 and now my sessions don't work" messages 
already.

Using req.get_sessions() addresses 2 issues:

1. Gets the current session instance if it exists, thus avoiding 
deadlock problems. The user is no longer forced to call sess.save() 
before calling PSP.run(), which makes their code simpler, as well as 
improving performance for any session class that uses the disk for 
persistence.

2. Creates a new session instance of *the correct type*, assuming a 
session class has be defined with PythonOption. This is bug in the 
current code, where a user could create a session in their code with 
something other than the default session class for their platform, for 
example "sess = Session.FileSession(req)". If psp needs to create a 
session it will be DbmSession on some platforms, and so the session data 
will be lost. Although now that I think about it, I wonder why I didn't 
just modify Session.Session to handle this part...  Hmm, I'll have to 
look at my notes.

And misses these issues:
3. Should the session automatically be saved in psp run()?
    Yes: - it's the current behavoiur
         - using sessions with psp pages is transparent
         - the user does not need to remember to do this
    No:  - the code monkey has less control over their application
         - ?
    Maybe we just need a attribute in the session to enable/disable 
saving? Default would be to save the session.

4. Should the session be unlocked in psp run()?
    Yes: - it's the current behavoiur
    No:  - since we are now passing around the session instance with the 
request, the session will be unlocked when the request is completed.

Regards,
Jim