Various session fixes, including #717 (sessions should have a __len__ function).

2 files changed, 35 insertions, 11 deletions

diff --git a/cherrypy/lib/sessions.py b/cherrypy/lib/sessions.py
index 89d4e48b..2a46627e 100644
--- a/cherrypy/lib/sessions.py
+++ b/cherrypy/lib/sessions.py
@@ -462,7 +462,13 @@ class PostgresqlSession(Session):
 
 class MemcachedSession(Session):
     
+    # The most popular memcached client for Python isn't thread-safe.
+    # Wrap all .get and .set operations in a single lock.
+    mc_lock = threading.RLock()
+    
+    # This is a seperate set of locks per session id.
     locks = {}
+    
     servers = ['127.0.0.1:11211']
     
     def setup(cls, **kwargs):
@@ -479,15 +485,21 @@ class MemcachedSession(Session):
     setup = classmethod(setup)
     
     def _load(self):
-        return self.cache.get(self.id)
+        self.mc_lock.acquire()
+        try:
+            return self.cache.get(self.id)
+        finally:
+            self.mc_lock.release()
     
     def _save(self, expiration_time):
         # Send the expiration time as "Unix time" (seconds since 1/1/1970)
-        zeroday = datetime.datetime(1970, 1, 1, tzinfo=expiration_time.tzinfo)
-        td = expiration_time - zeroday
-        td = (td.days * 86400) + td.seconds
-        if not self.cache.set(self.id, (self._data, expiration_time), td):
-            raise AssertionError("Session data for id %r not set." % self.id)
+        td = int(time.mktime(expiration_time.timetuple()))
+        self.mc_lock.acquire()
+        try:
+            if not self.cache.set(self.id, (self._data, expiration_time), td):
+                raise AssertionError("Session data for id %r not set." % self.id)
+        finally:
+            self.mc_lock.release()
     
     def _delete(self):
         self.cache.delete(self.id)
@@ -501,6 +513,10 @@ class MemcachedSession(Session):
         """Release the lock on the currently-loaded session data."""
         self.locks[self.id].release()
         self.locked = False
+    
+    def __len__(self):
+        """Return the number of active sessions."""
+        raise NotImplementedError
 
 
 # Hook functions (for CherryPy tools)
@@ -531,7 +547,7 @@ save.failsafe = True
 def close():
     """Close the session object for this request."""
     sess = getattr(cherrypy.serving, "session", None)
-    if sess and sess.locked:
+    if getattr(sess, "locked", False):
         # If the session is still locked we release the lock
         sess.release_lock()
 close.failsafe = True
diff --git a/cherrypy/test/test_session.py b/cherrypy/test/test_session.py
index 22581b94..927d79f4 100755
--- a/cherrypy/test/test_session.py
+++ b/cherrypy/test/test_session.py
@@ -115,6 +115,7 @@ class SessionTest(helper.CPWebCase):
                 os.unlink(os.path.join(localDir, fname))
     
     def test_0_Session(self):
+        self.getPage('/setsessiontype/ram')
         self.getPage('/clear')
         
         self.getPage('/testStr')
@@ -188,6 +189,7 @@ class SessionTest(helper.CPWebCase):
         cookies = self.cookies
         
         data_dict = {}
+        errors = []
         
         def request(index):
             if self.scheme == 'https':
@@ -200,12 +202,13 @@ class SessionTest(helper.CPWebCase):
                     c.putheader(k, v)
                 c.endheaders()
                 response = c.getresponse()
-                self.assertEqual(response.status, 200)
                 body = response.read()
-                self.failUnless(body.isdigit())
+                if response.status != 200 or not body.isdigit():
+                    errors.append((response.status, body))
+                else:
+                    data_dict[index] = max(data_dict[index], int(body))
                 # Uncomment the following line to prove threads overlap.
 ##                print index,
-            data_dict[index] = max(data_dict[index], int(body))
         
         # Start <request_count> requests from each of
         # <client_thread_count> concurrent clients
@@ -221,6 +224,9 @@ class SessionTest(helper.CPWebCase):
         
         hitcount = max(data_dict.values())
         expected = 1 + (client_thread_count * request_count)
+        
+        for e in errors:
+            print e
         self.assertEqual(hitcount, expected)
     
     def test_3_Redirect(self):
@@ -309,6 +315,9 @@ else:
             self.assertBody('2')
             self.getPage('/testStr', self.cookies)
             self.assertBody('3')
+            self.getPage('/length', self.cookies)
+            self.assertErrorPage(500)
+            self.assertInBody("NotImplementedError")
             self.getPage('/delkey?key=counter', self.cookies)
             self.assertStatus(200)
             
@@ -381,7 +390,6 @@ else:
 
 
 
-
 if __name__ == "__main__":
     setup_server()
     helper.testmain()