Graham Dumpleton wrote ..
>
> On 09/08/2005, at 7:51 AM, Graham Dumpleton wrote:
> >> Will it introduce new and exciting locking issues for accessing the
> >> cache?
> >
> > I don't think so, the caching is effectively done on a thread specific
> > basis only and thus where the lock synchronisation point for different
> > threads wanting to create the same session object doesn't matter. The
> > whole point of the cache is only to deal with the one thread which does
> > an req.internal_redirect() and wants to access the session object with
> > that redirect as well.
>
> Whoops, it would introduce a single locking point around creation of all
> session objects from Session() function, ie., from any thread. This may
> have a performance impact. With some creative coding this might be
> avoided though. I'll think about it some more. :-)
Actually, no locking problems after all that I can see. I am going to be busy
giving training most of the day, but wacked together the following patches
on the train on the way to work. I tested it for simple case with no actual
internal redirect, but didn't have time to check it for internal redirect. :-(
Anyway, should give you a better idea of what I am talking about. The
changes are actually quite simple. Although, if someone doesn't use the
Session.Session() function and uses a specific class direct, then it bypasses
the code. That was the same for req.get_session() as well though.
Patch below for viewing online, but also added as attachment. Note, all
changes are in Session.py. There is no need to modify the request object
at all, ie., no req.get_session() and no changes to req.internal_redirect().
Index: lib/python/mod_python/Session.py
===================================================================
--- lib/python/mod_python/Session.py (revision 230781)
+++ lib/python/mod_python/Session.py (working copy)
@@ -37,6 +37,15 @@
tempdir = tempfile.gettempdir()
+try:
+ import threading
+ _current_thread = threading.currentThread
+except:
+ def _current_thread():
+ return None
+
+_per_request_cache = {}
+
def _init_rnd():
""" initialize random number generators
this is key in multithreaded env, see
@@ -220,9 +229,14 @@
_apache._global_lock(self._req.server, self._sid)
self._locked = 1
self._req.register_cleanup(unlock_session_cleanup, self)
+ _per_request_cache[(_current_thread(),self._sid)] = self
def unlock(self):
if self._lock and self._locked:
+ try:
+ del _per_request_cache[(_current_thread(),self._sid)]
+ except:
+ pass
_apache._global_unlock(self._req.server, self._sid)
self._locked = 0
@@ -676,6 +690,27 @@
# For now, just raise an exception.
raise Exception, 'Unknown session type %s' % sess_type
+ # Following check for sid duplicates what BaseSession does
+ # and overrides it, but need it so we can look up cache.
+ session_cookie_name =
req.get_options().get("session_cookie_name",COOKIE_NAME)
+ if not sid:
+ # check to see if cookie exists
+ if secret:
+ cookies = Cookie.get_cookies(req, Class=Cookie.SignedCookie,
+ secret=secret)
+ else:
+ cookies = Cookie.get_cookies(req)
+
+ if cookies.has_key(session_cookie_name):
+ sid = cookies[session_cookie_name].value
+
+ session = _per_request_cache.get((_current_thread(),sid),None)
+ if session:
+ # XXX Should possibly raise an exception here if type of
+ # cached session object is different to "sess". Would
+ # probably indicate an issue with users code if different.
+ return session
+
return sess(req, sid=sid, secret=secret,
timeout=timeout, lock=lock)
Index: lib/python/mod_python/Session.py
===================================================================
--- lib/python/mod_python/Session.py (revision 230781)
+++ lib/python/mod_python/Session.py (working copy)
@@ -37,6 +37,15 @@
tempdir = tempfile.gettempdir()
+try:
+ import threading
+ _current_thread = threading.currentThread
+except:
+ def _current_thread():
+ return None
+
+_per_request_cache = {}
+
def _init_rnd():
""" initialize random number generators
this is key in multithreaded env, see
@@ -220,9 +229,14 @@
_apache._global_lock(self._req.server, self._sid)
self._locked = 1
self._req.register_cleanup(unlock_session_cleanup, self)
+ _per_request_cache[(_current_thread(),self._sid)] = self
def unlock(self):
if self._lock and self._locked:
+ try:
+ del _per_request_cache[(_current_thread(),self._sid)]
+ except:
+ pass
_apache._global_unlock(self._req.server, self._sid)
self._locked = 0
@@ -676,6 +690,27 @@
# For now, just raise an exception.
raise Exception, 'Unknown session type %s' % sess_type
+ # Following check for sid duplicates what BaseSession does
+ # and overrides it, but need it so we can look up cache.
+ session_cookie_name =
req.get_options().get("session_cookie_name",COOKIE_NAME)
+ if not sid:
+ # check to see if cookie exists
+ if secret:
+ cookies = Cookie.get_cookies(req, Class=Cookie.SignedCookie,
+ secret=secret)
+ else:
+ cookies = Cookie.get_cookies(req)
+
+ if cookies.has_key(session_cookie_name):
+ sid = cookies[session_cookie_name].value
+
+ session = _per_request_cache.get((_current_thread(),sid),None)
+ if session:
+ # XXX Should possibly raise an exception here if type of
+ # cached session object is different to "sess". Would
+ # probably indicate an issue with users code if different.
+ return session
+
return sess(req, sid=sid, secret=secret,
timeout=timeout, lock=lock)
