Session expiry check frequency should be based on sessionTimeout
|Reported by:||mthuurne||Owned by:|
branch-diff, diff-cov, branch-cov, buildbot
Currently, there are two time intervals that play a role in session expiry: Site.sessionCheckTime and Session.sessionTimeout. In Twisted 2.5, the check time and timeout were hardcoded. Since Twisted 8.0, they are customizable. Ticket #1090 describes the actions taken to get rid of the hardcoded values.
The values of sessionCheckTime and sessionTimeout are not tied together in any way. This means that if you increase the sessionTimeout, session expiry is checked unnecessarily often. In the opposite case, sessions with a short sessionTimeout will not be cleaned up faster than the sessionCheckTime. Neither of this is a problem by itself: the default value of sessionCheckTime is 1800 and one check per 30 minutes is not likely to become a performance problem, while keeping expired sessions around for at most 30 minutes after they expired is also unlikely to consume a significant amount of memory.
However, it is possible to register a routine to be called when a session expires using Session.notifyOnExpire(). The current implementation will call the callback when the session is cleaned up, not when it expires. If for example sessionTimeout is set to 5 minutes and sessionCheckTime to 30 minutes, the expiry notification will be 25 minutes late. This could be a problem if for example you want to show a list of logged in users, since you will see users listed that did not log out, but are no longer logged in either because their session has expired.
The expiry check should be done as short as possible after the expiry time. One way of doing this would be to have a check frequency per Session instead of per Site, which is calculated from the sessionTimeout. Another way would be to use a kind of resettable watchdog timer instead of a periodic check: every time the session is used (Session.touch()), reset the timer to the timeout value. In either case, Site.sessionCheckTime would serve no function, so it could be deprecated or removed.
Change History (9)
comment:2 Changed 8 years ago by
|Owner:||changed from jknight to Jean-Paul Calderone|
|Status:||new → assigned|
comment:3 Changed 8 years ago by
comment:4 Changed 8 years ago by
|Owner:||Jean-Paul Calderone deleted|
|Status:||assigned → new|
comment:5 follow-up: 6 Changed 8 years ago by
|Cc:||Michael Hudson-Doyle added|
|Owner:||set to Jean-Paul Calderone|