On Mar 14, 2011, at 8:55 AM, Corne wrote:
>
> We've looked into the problem a bit deeper.. There is a problem but
> it's different than what was in the first post.
>
> The problem seems to be in the locking of the session file:
> Two processes/requests open the same session file for write and thus
> get a handle on it
>
> One request gets the portalocker lock on the session file, the other
> one blocks and waits for the first request to release the lock.
>
> The first request writes the changes to the session file, and releases
> the lock.
> The second request then gets the lock, but still uses the old file
> handle and results in an EOFError exception
Something else (or something more) is going on here, I think. There's nothing
wrong with using the "old" file handle, at least not that I can see. I wrote a
simplified test case to verify that (which I could have wrong, but I don't
think so).
Something that bothers me a bit (though I don't think it's directly relevant to
this problem) is the lazy creation of the session file. If the session file
doesn't exist, it doesn't get created until the end of the request, when the
session file is to be updated. If the current request calls session.forget,
then it doesn't get created, but it sends back a cookie with the new session
id. The next request sees that the file is missing and generates a new session
id. So until we get a request that doesn't forget the session, we keep creating
new session ids and sending them back.
Also, if a client (presumably not a browser, which I can't see doing this)
sends two simultaneous requests before the session file gets created, there's a
race condition such that they'll get different session ids. Now, that'd be
fairly strange behavior, but (I think) possible. The liberal use of
session.forget could make it more likely.
Maybe it's not worth worrying about.
>
>
> The first patch improved the situation, because the file is re-opened
> after the file is released.
> However, a proper way to handle this would be to use a lock file,
> which is used only for locking and not for storing data at the same
> time, as per attached patch
>
>
>
>
> --- globals.py 2011-03-14 16:17:19.466604041 +0100
> +++ globals.py_new 2011-03-11 16:55:33.000000000 +0100
> @@ -269,10 +269,12 @@
> response.session_id = None
> if response.session_id:
> try:
> + response.session_lock = \
> + open(response.session_filename+'.lock', 'wb')
> + portalocker.lock(response.session_lock,
> + portalocker.LOCK_EX)
> response.session_file = \
> open(response.session_filename, 'rb+')
> - portalocker.lock(response.session_file,
> - portalocker.LOCK_EX)
> self.update(cPickle.load(response.session_file))
> response.session_file.seek(0)
> oc = response.session_filename.split('/')
> [-1].split('-')[0]
> @@ -395,21 +397,23 @@
> session_folder =
> os.path.dirname(response.session_filename)
> if not os.path.exists(session_folder):
> os.mkdir(session_folder)
> + response.session_lock = open(response.session_filename
> +'.lock', 'wb')
> + portalocker.lock(response.session_lock,
> portalocker.LOCK_EX)
> response.session_file = open(response.session_filename,
> 'wb')
> - portalocker.lock(response.session_file,
> portalocker.LOCK_EX)
> if response.session_file:
> cPickle.dump(dict(self), response.session_file)
> response.session_file.truncate()
> try:
> - portalocker.unlock(response.session_file)
> + portalocker.unlock(response.session_lock)
> response.session_file.close()
> + del response.session_lock
> del response.session_file
> except: ### this should never happen but happens in
> Windows
> pass
>
> def _unlock(self, response):
> - if response and response.session_file:
> + if response and response.session_file and
> response.session_lock:
> try:
> - portalocker.unlock(response.session_file)
> + portalocker.unlock(response.session_lock)
> except: ### this should never happen but happens in
> Windows
> pass
