Re: [jira] Commented: (MODPYTHON-59) Add get_session() method torequestobject

[Graham Dumpleton <gr...@dscpl.com.au>]

Jim Gallacher wrote ..
> I think the automatic session unlock needs to stay. It's just too easy
> for the user to forget a manual unlock, deadlock the session and hose 
> their server in very short order.

What though is the consequence of the session lock not being reacquired.
Ie., what happens if someone wants to modify the session object and save
it after internal redirect returns.

You might save a deadlock by unlocking the session object, but does it
screw up the case of existing code where session is modified and saved
after internal redirect returns.

Am still for leaving things how they are in this respect, we know the
issues involved and probably just need to beef up the documentation with
big warnings so that people are aware of the internal redirect caveat.

Graham

Re: [jira] Commented: (MODPYTHON-59) Add get_session() method torequestobject

[Jim Gallacher <jg...@sympatico.ca>]

Graham Dumpleton wrote:
> Jim Gallacher wrote ..
> 
>>I think the automatic session unlock needs to stay. It's just too easy
>>for the user to forget a manual unlock, deadlock the session and hose 
>>their server in very short order.
> 

In your following comments I'm assuming you are talking about the case 
where the code called by internal_redirect is *not* creating a session.

> What though is the consequence of the session lock not being reacquired.
> Ie., what happens if someone wants to modify the session object and save
> it after internal redirect returns.

They can still do that. Nothing will break, but we have introduced a 
possible race condition if another request has come in for the same 
session in the interim. Note that this race condition currently exists 
where the code explicitly unlocks the session.

It may be tricky for the user to track down what is going on but I think 
this is better than potentially exhausting all the mutexes through 
deadlocks and needing to restart apache. A programming error in one app 
should not be allowed to bring down the whole server. To me restarting 
the server is the bigger sin.

> You might save a deadlock by unlocking the session object, but does it
> screw up the case of existing code where session is modified and saved
> after internal redirect returns.

Current code will not break. Assuming that another request for the same 
session has not arrived in the interim and changed the underlying 
session data, all is well.

One possible solution would be to add an optional parameter to 
req_internal_redirect:

internal_redirect(url, unlock_session=True)

Current code which messes with the session object after 
internal_redirect returns would still need to be modified, but at least 
the programmer has that control and we avoid deadlocks by default.

> Am still for leaving things how they are in this respect, we know the
> issues involved and probably just need to beef up the documentation with
> big warnings so that people are aware of the internal redirect caveat.

People actually read the documention? Cool. ;)

I still advocate for automatic session unlocking for an internal 
redirect. The messages on the mailing list where people have suggested 
not locking sessions at all to the avoid deadlock problems suggests to 
me that session locking needs to handled much more transparently. Also, 
at some point someone (Grisha?) suggested that the likes of php sessions 
may not do any session locking, so we are still ahead of the game.

Any other opinions?

Regards,
Jim