[CherryPy] #1082: RamSession leak

[CherryPy]

#1082: RamSession leak
------------------------------------------------+---------------------------
Reporter: frans.van.n...@ingbank.com | Owner: no_mind
Type: defect | Status: new
Priority: normal | Milestone:
Component: sessions | Keywords:
------------------------------------------------+---------------------------
Every request on a cherrypy server using RamSession
('tools.sessions.storage_type': 'ram') will cause a memory leak. I've
noticed this because our production intranet server machine running
cherrypy became unresponsive. It turned out that the cherrypy server was
leaking handles.

Diving into the problem showed that RamSession released its lock every
time, but kept the threading.RLock object in its 'self.locks' dictionary.

So, the leak defect can be resolved by changing the implementation of
RamSession.release_lock to:

def release_lock(self):
"""Release the lock on the currently-loaded session data."""
self.locks[self.id].release()
del self.locks[self.id] # add this to remove memory the leak
self.locked = False

--
Ticket URL: <http://www.cherrypy.org/ticket/1082>
CherryPy <http://www.cherrypy.org>
CherryPy - a pythonic, object-oriented HTTP framework

[CherryPy]

#1082: RamSession leak
------------------------------------------------+---------------------------
Reporter: frans.van.n...@ingbank.com | Owner: no_mind
Type: defect | Status: new
Priority: normal | Milestone:

Component: sessions | Resolution:
Keywords: |
------------------------------------------------+---------------------------
Comment (by frans.van.n...@ingbank.com):

I've dived into the problem a little deeper.

In fact, the leak is limited to requests that don't store any data in a
session. Reason is, that the clean_up method works based on sessions in
the RamSession.cache dictionary (which only holds sessions that contain
data). The RamSession.locks dictionary however, also contains items for
sessions that don't store any data. Thus, these will never get deleted.

In contrast to what I suggested earlier in this ticket (sorry about that),
the best solution to this problem is therefore probably an addition
something like the following to the clean_up method:

{{{
#!python
def clean_up(self):
"""Clean up expired sessions."""
now = datetime.datetime.now()
for id, (data, expiration_time) in copyitems(self.cache):
if expiration_time <= now:
try:
del self.cache[id]
except KeyError:
pass
try:
del self.locks[id]
except KeyError:
pass
# added to remove obsolete lock objects
ids = self.locks.keys()
for id in ids:
try:
if not self.cache.has_key(id):
del self.locks[id]
except:
pass
}}}

While running the tests, I came across a problem in the test code. In
test_session.py, the following line

{{{
#!python
self.assertErrorPage(405, response_codes[405])
}}}

raises an exception because the assertErrorPage expects a string while the
response_codes dictionary contains tuples.
I suggest changing it to

{{{
#!python
self.assertErrorPage(405)
}}}

--
Ticket URL: <http://cherrypy.org/ticket/1082>