Good catch. Thanks Jonathan.

On Jan 19, 12:45 pm, Jonathan Lundell <jlund...@pobox.com> wrote:
> On Jan 19, 2011, at 10:26 AM, ron_m wrote:
>
> > If the response is left off the session.forget() parameter list it defaults 
> > to None. The end result then is the session._forget state variable is set 
> > True but the session file is not unlocked in the _unlock function. This 
> > would enhance performance by bypassing the writing of the session file at 
> > the end of the request-response cycle when the session state is unchanged 
> > but won't release the lock to let other requests proceed on that session. I 
> > could see this as a possible use of session.forget(..) so it looks like a 
> > documentation issue.
>
> There's another odd behavior. If you call session.forget(response), then 
> _unlock will eventually be called twice (once for forget, once in the 
> early-out path of _try_store_on_disk).
>
>     def _unlock(self, response):
>         if response and response.session_file:
>             try:
>                 portalocker.unlock(response.session_file)
>             except: ### this should never happen but happens in Windows
>                 pass
>
> Perhaps that's the reason for the "should never happen" exception?
>
> Possible fix:
>
>     def _unlock(self, response):
>         if response and response.session_file:
>             try:
>                 portalocker.unlock(response.session_file)
>             except: ### this should never happen but happens in Windows
>                 pass
>             response.session_file.close()
>             del response.session_file
>
> The logic is subtle, because Session defers the creation of the original 
> session file until _try_store_on_disk, so if you forget() a session before 
> the file is created, it never gets created. I think this logic works, though, 
> assuming that a double unlock happens only because of forget.

Reply via email to