# HG changeset patch # User Sylvain Thénault # Date 1280138904 -7200 # Node ID d56fd78006cd1888c26182a28b11c943622ad1c2 # Parent b5f15098f282216cb91a5c507c0dc49e074a7eae [session] cleanup session-time / cleanup-session-time... which are hard to grasp while there is no actual benefit to handle both. Now repo will close session without any activity since cleanup-session-time (24h by default), and the web authentication chain won't reconnect automatically anymore. I don't think there is a big deal in keeping repo session for such time. Also, ask the repo for latest session usage time, we can't know it for real on the web side (think of long running transactions). Closes #1083245. diff -r b5f15098f282 -r d56fd78006cd dbapi.py --- a/dbapi.py Mon Jul 26 12:07:00 2010 +0200 +++ b/dbapi.py Mon Jul 26 12:08:24 2010 +0200 @@ -507,10 +507,12 @@ return DBAPIRequest(self.vreg, DBAPISession(self)) def check(self): - """raise `BadConnectionId` if the connection is no more valid""" + """raise `BadConnectionId` if the connection is no more valid, else + return its latest activity timestamp. + """ if self._closed is not None: raise ProgrammingError('Closed connection') - self._repo.check_session(self.sessionid) + return self._repo.check_session(self.sessionid) def set_session_props(self, **props): """raise `BadConnectionId` if the connection is no more valid""" diff -r b5f15098f282 -r d56fd78006cd etwist/twconfig.py --- a/etwist/twconfig.py Mon Jul 26 12:07:00 2010 +0200 +++ b/etwist/twconfig.py Mon Jul 26 12:08:24 2010 +0200 @@ -76,12 +76,6 @@ the repository rather than the user running the command', 'group': 'main', 'level': WebConfiguration.mode == 'system' }), - ('session-time', - {'type' : 'time', - 'default': '30min', - 'help': 'session expiration time, default to 30 minutes', - 'group': 'main', 'level': 1, - }), ('pyro-server', {'type' : 'yn', # pyro is only a recommends by default, so don't activate it here diff -r b5f15098f282 -r d56fd78006cd misc/migration/3.10.0_common.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/misc/migration/3.10.0_common.py Mon Jul 26 12:08:24 2010 +0200 @@ -0,0 +1,1 @@ +option_group_changed('cleanup-session-time', 'web', 'main') diff -r b5f15098f282 -r d56fd78006cd server/repository.py --- a/server/repository.py Mon Jul 26 12:07:00 2010 +0200 +++ b/server/repository.py Mon Jul 26 12:08:24 2010 +0200 @@ -267,7 +267,10 @@ # call instance level initialisation hooks self.hm.call_hooks('server_startup', repo=self) # register a task to cleanup expired session - self.looping_task(self.config['session-time']/3., self.clean_sessions) + self.cleanup_session_time = self.config['cleanup-session-time'] or 60 * 60 * 24 + assert self.cleanup_session_time > 0 + cleanup_session_interval = min(60*60, self.cleanup_session_time / 3) + self.looping_task(cleanup_session_interval, self.clean_sessions) assert isinstance(self._looping_tasks, list), 'already started' for i, (interval, func, args) in enumerate(self._looping_tasks): self._looping_tasks[i] = task = utils.LoopTask(interval, func, args) @@ -622,8 +625,10 @@ session.reset_pool() def check_session(self, sessionid): - """raise `BadConnectionId` if the connection is no more valid""" - self._get_session(sessionid, setpool=False) + """raise `BadConnectionId` if the connection is no more valid, else + return its latest activity timestamp. + """ + return self._get_session(sessionid, setpool=False).timestamp def get_shared_data(self, sessionid, key, default=None, pop=False): """return the session's data dictionary""" @@ -771,7 +776,7 @@ """close sessions not used since an amount of time specified in the configuration """ - mintime = time() - self.config['session-time'] + mintime = time() - self.cleanup_session_time self.debug('cleaning session unused since %s', strftime('%T', localtime(mintime))) nbclosed = 0 diff -r b5f15098f282 -r d56fd78006cd server/serverconfig.py --- a/server/serverconfig.py Mon Jul 26 12:07:00 2010 +0200 +++ b/server/serverconfig.py Mon Jul 26 12:08:24 2010 +0200 @@ -120,10 +120,16 @@ the repository rather than the user running the command', 'group': 'main', 'level': (CubicWebConfiguration.mode == 'installed') and 0 or 1, }), - ('session-time', + ('cleanup-session-time', {'type' : 'time', - 'default': '30min', - 'help': 'session expiration time, default to 30 minutes', + 'default': '24h', + 'help': 'duration of inactivity after which a session ' + 'will be closed, to limit memory consumption (avoid sessions that ' + 'never expire and cause memory leak when http-session-time is 0, or ' + 'because of bad client that never closes their connection). ' + 'So notice that even if http-session-time is 0 and the user don\'t ' + 'close his browser, he will have to reauthenticate after this time ' + 'of inactivity. Default to 24h.', 'group': 'main', 'level': 3, }), ('connections-pool-size', diff -r b5f15098f282 -r d56fd78006cd web/application.py --- a/web/application.py Mon Jul 26 12:07:00 2010 +0200 +++ b/web/application.py Mon Jul 26 12:08:24 2010 +0200 @@ -31,7 +31,7 @@ from cubicweb import set_log_methods, cwvreg from cubicweb import ( ValidationError, Unauthorized, AuthenticationError, NoSelectableObject, - RepositoryError, CW_EVENT_MANAGER) + RepositoryError, BadConnectionId, CW_EVENT_MANAGER) from cubicweb.dbapi import DBAPISession from cubicweb.web import LOGGER, component from cubicweb.web import ( @@ -48,48 +48,43 @@ def __init__(self, vreg): self.session_time = vreg.config['http-session-time'] or None - if self.session_time is not None: - assert self.session_time > 0 - self.cleanup_session_time = self.session_time - else: - self.cleanup_session_time = vreg.config['cleanup-session-time'] or 1440 * 60 - assert self.cleanup_session_time > 0 - self.cleanup_anon_session_time = vreg.config['cleanup-anonymous-session-time'] or 5 * 60 - assert self.cleanup_anon_session_time > 0 self.authmanager = vreg['components'].select('authmanager', vreg=vreg) + interval = (self.session_time or 0) / 2. if vreg.config.anonymous_user() is not None: - self.clean_sessions_interval = max( - 5 * 60, min(self.cleanup_session_time / 2., - self.cleanup_anon_session_time / 2.)) - else: - self.clean_sessions_interval = max( - 5 * 60, - self.cleanup_session_time / 2.) + self.cleanup_anon_session_time = vreg.config['cleanup-anonymous-session-time'] or 5 * 60 + assert self.cleanup_anon_session_time > 0 + if self.session_time is not None: + self.cleanup_anon_session_time = min(self.session_time, + self.cleanup_anon_session_time) + interval = self.cleanup_anon_session_time / 2. + # we don't want to check session more than once every 5 minutes + self.clean_sessions_interval = max(5 * 60, interval) def clean_sessions(self): """cleanup sessions which has not been unused since a given amount of time. Return the number of sessions which have been closed. """ self.debug('cleaning http sessions') + session_time = self.session_time closed, total = 0, 0 for session in self.current_sessions(): - no_use_time = (time() - session.last_usage_time) total += 1 - if session.anonymous_session: - if no_use_time >= self.cleanup_anon_session_time: + try: + last_usage_time = session.cnx.check() + except BadConnectionId: + self.close_session(session) + closed += 1 + else: + no_use_time = (time() - last_usage_time) + if session.anonymous_session: + if no_use_time >= self.cleanup_anon_session_time: + self.close_session(session) + closed += 1 + elif session_time is not None and no_use_time >= session_time: self.close_session(session) closed += 1 - elif no_use_time >= self.cleanup_session_time: - self.close_session(session) - closed += 1 return closed, total - closed - def has_expired(self, session): - """return True if the web session associated to the session is expired - """ - return not (self.session_time is None or - time() < session.last_usage_time + self.session_time) - def current_sessions(self): """return currently open sessions""" raise NotImplementedError() @@ -213,8 +208,6 @@ except AuthenticationError: req.remove_cookie(cookie, self.SESSION_VAR) raise - # remember last usage time for web session tracking - session.last_usage_time = time() def get_session(self, req, sessionid): return self.session_manager.get_session(req, sessionid) @@ -224,8 +217,6 @@ cookie = req.get_cookie() cookie[self.SESSION_VAR] = session.sessionid req.set_cookie(cookie, self.SESSION_VAR, maxage=None) - # remember last usage time for web session tracking - session.last_usage_time = time() if not session.anonymous_session: self._postlogin(req) return session diff -r b5f15098f282 -r d56fd78006cd web/views/authentication.py --- a/web/views/authentication.py Mon Jul 26 12:07:00 2010 +0200 +++ b/web/views/authentication.py Mon Jul 26 12:08:24 2010 +0200 @@ -74,7 +74,7 @@ self.repo = vreg.config.repository(vreg) self.log_queries = vreg.config['query-log-file'] self.authinforetreivers = sorted(vreg['webauth'].possible_objects(vreg), - key=lambda x: x.order) + key=lambda x: x.order) # 2-uple login / password, login is None when no anonymous access # configured self.anoninfo = vreg.config.anonymous_user() @@ -98,25 +98,11 @@ if login and session.login != login: raise InvalidSession('login mismatch') try: - lock = session.reconnection_lock - except AttributeError: - lock = session.reconnection_lock = Lock() - # need to be locked two avoid duplicated reconnections on concurrent - # requests - with lock: - cnx = session.cnx - try: - # calling cnx.user() check connection validity, raise - # BadConnectionId on failure - user = cnx.user(req) - except BadConnectionId: - # check if a connection should be automatically restablished - if (login is None or login == session.login): - cnx = self._authenticate(session.login, session.authinfo) - user = cnx.user(req) - session.cnx = cnx - else: - raise InvalidSession('bad connection id') + # calling cnx.user() check connection validity, raise + # BadConnectionId on failure + user = session.cnx.user(req) + except BadConnectionId: + raise InvalidSession('bad connection id') return user def authenticate(self, req): diff -r b5f15098f282 -r d56fd78006cd web/views/debug.py --- a/web/views/debug.py Mon Jul 26 12:07:00 2010 +0200 +++ b/web/views/debug.py Mon Jul 26 12:08:24 2010 +0200 @@ -118,10 +118,15 @@ if sessions: w(u'') diff -r b5f15098f282 -r d56fd78006cd web/views/sessions.py --- a/web/views/sessions.py Mon Jul 26 12:07:00 2010 +0200 +++ b/web/views/sessions.py Mon Jul 26 12:08:24 2010 +0200 @@ -17,8 +17,8 @@ # with CubicWeb. If not, see . """web session component: by dfault the session is actually the db connection object :/ +""" -""" __docformat__ = "restructuredtext en" from cubicweb.web import InvalidSession @@ -51,9 +51,6 @@ if not sessionid in self._sessions: raise InvalidSession() session = self._sessions[sessionid] - if self.has_expired(session): - self.close_session(session) - raise InvalidSession() try: user = self.authmanager.validate_session(req, session) except InvalidSession: diff -r b5f15098f282 -r d56fd78006cd web/webconfig.py --- a/web/webconfig.py Mon Jul 26 12:07:00 2010 +0200 +++ b/web/webconfig.py Mon Jul 26 12:08:24 2010 +0200 @@ -135,17 +135,6 @@ "Should be 0 or greater than repository\'s session-time.", 'group': 'web', 'level': 2, }), - ('cleanup-session-time', - {'type' : 'time', - 'default': '24h', - 'help': 'duration of inactivity after which a connection ' - 'will be closed, to limit memory consumption (avoid sessions that ' - 'never expire and cause memory leak when http-session-time is 0). ' - 'So even if http-session-time is 0 and the user don\'t close his ' - 'browser, he will have to reauthenticate after this time of ' - 'inactivity. Default to 24h.', - 'group': 'web', 'level': 3, - }), ('cleanup-anonymous-session-time', {'type' : 'time', 'default': '5min',