SessionDynamicStore memoryKeysInAccessTimeOrder - RuntimeError: dictionary changed size during iteration about w4py HOT 4 CLOSED

Thanks for reporting, I see the problem:

    accessTimeAndKeys = []
    for key in self._memoryStore:
        try:
            accessTimeAndKeys.append(
                (self._memoryStore[key].lastAccessTime(), key))
        except KeyError:
            pass
    return map(itemgetter(1), sorted(accessTimeAndKeys))

If another thread (like the session sweeper) deletes a session from the store while the for loop is running, then this will fail with a "dictionary changed size" Runtime error. Catching KeyError will not help here. With your patch, this will work because a KeyError is raised in this case.

Maybe the following would be a bit more elegant as it's shorter and avoids catching the KeyError?

    return [item[0] for item in sorted((session.lastAccessTime(), key)
        for key, session in self._memoryStore.items())]

Or maybe this is even more elegant:

    return [session.identifier() for session in sorted(
        self._memoryStore.values(), key=lambda v: v.lastAccessTime())]

We could run the loop inside with self._lock as we do in other methods, but that would hurt performance.

from w4py.

I like your last option. It's compact and DRY, because the base definition of SessionStore.values() already has the loop with an internal try...except to catch missing keys.

Note that there are other places in SessionDynamicStore which also iterate over self.keys(). I don't think it's a construct that needs to be eliminated entirely, especially within a SessionStore that manages it's own internals.

edit ---> So as soon as I clicked "Update comment" I realized that the base definitions of SessionStore.values() and SessionStore.items() both appear to have the same bug as they iterate over self instead of self.keys()

from w4py.

... realized that the base definitions of SessionStore.values() and SessionStore.items() both appear to have the same bug ...

Fixed this in a second commit. There was also another one in SessionMemoryStore. Can you confirm that everything is fixed with the last two commits?

from w4py.

Looks good to me and a better overall fix than my initial submission. Thanks!

from w4py.