[ http://issues.apache.org/jira/brows...n_66502
 ]

Nicolas Lehuen commented on MODPYTHON-57:
-----------------------------------------

We also have to protect against the case where some sessions are added to th
e sdict dictionary while the cleanup is being performed, or even against the
 case where two cleanups are performed concurrently. Hence, the same locking
 mechanisms that were used
on DbmSession and FileSession should be applied here.

Fortunately we can rely on the GIL so that a call to sdict.keys() returns a 
snapshot of the SIDs at one point, so that even if some new session are adde
d, we won't have a "dictionary changed size during iteration" exception, the
n protect against KeyError
with a try..except block, in case another cleanup has deleted the session wh
ile we were examining it.

The final version is therefore :

def mem_cleanup(sdict):
for sid in sdict.keys():
try:
session = sdict[sid]
if (time.time() - session["_accessed"]) > session["_timeout"]:
del sdict[sid]
except:
pass

This will do the trick, but a really clean implementation would rely on glob
al locks, just like DbmSession and FileSession do.


> mem_cleanup throws "dictionary changed size during iteration"
> -------------------------------------------------------------
>
>          Key: MODPYTHON-57
>          URL: http://issues.apache.org/jira/browse/MODPYTHON-57
>      Project: mod_python
>         Type: Bug
>   Components: core
>     Versions: 3.1.4
>  Environment: We are running under Windows, but the problem is likely not 
O/S dependent.
>     Reporter: Shack Toms
>     Priority: Minor
>      Fix For: 3.2.0

>
> The mem_cleanup routine, in Session.py, appears to have a bug that causes
> Python to throw "dictionary changed size during iteration".
> The current code is.
> def mem_cleanup(sdict):
>     for sid in sdict:
>         dict = sdict[sid]
>         if (time.time() - dict["_accessed"]) > dict["_timeout"]:
>             del sdict[sid]
> The for statement should be changed to
>     for sid in sdict.keys():
> This will first generate a list of the keys of the sdict, and will avoid t
he
> complaint about the dictionary changing on the "del sdict[sid]" statem
ent.
> The dbm_cleanup code has another approach, to use one iteration to gather
> the keys to be deleted, and then have a second iteration over the gathered
> keys which does the del.   Using that approach, mem_cleanup should become.
.
> def mem_cleanup(sdict):
>     old = []
>     for sid in sdict:
>         dict = sdict[sid]
>         if (time.time() - dict["_accessed"]) > dict["_timeout"]:
>             old.append(sid)
>     for sid in old:
>         del sdict[sid]
> This is more efficient if the number of sessions is large compared to the
> number of sessions to be deleted.

[ Post a follow-up to this message ]