# HG changeset patch
# User Goffi <goffi@goffi.org>
# Date 1439669607 -7200
# Node ID b003dbd2b4e925b434558ac21a8b95a561f84c40
# Parent  e8c8e467964bf72a3bbe075fda4767a98afaf343
core (memory): Sessions fixes:
    - avoid memory leak with forgotten _purgeSession in case of conflict: an exception is now raised instead
    - _purgeSession now cancel the timer if it was not done before
    - KeyError in profileGet is now explicitly raised throught a failure.Failure
    - use iterator to iterate session in ProfileSessions

diff -r e8c8e467964b -r b003dbd2b4e9 src/memory/memory.py
--- a/src/memory/memory.py	Sat Aug 15 22:13:27 2015 +0200
+++ b/src/memory/memory.py	Sat Aug 15 22:13:27 2015 +0200
@@ -27,7 +27,8 @@
 from collections import namedtuple
 from ConfigParser import SafeConfigParser, NoOptionError, NoSectionError
 from uuid import uuid4
-from twisted.internet import defer, reactor
+from twisted.python import failure
+from twisted.internet import defer, reactor, error
 from twisted.words.protocols.jabber import jid
 from sat.core import exceptions
 from sat.core.constants import Const as C
@@ -40,6 +41,7 @@
 
 
 PresenceTuple = namedtuple("PresenceTuple", ('show', 'priority', 'statuses'))
+MSG_NO_SESSION = "Session id doesn't exist or is finished"
 
 class Sessions(object):
     """Sessions are data associated to key used for a temporary moment, with optional profile checking."""
@@ -55,7 +57,8 @@
         self.resettable_timeout = resettable_timeout
 
     def newSession(self, session_data=None, session_id=None, profile=None):
-        """ Create a new session
+        """Create a new session
+
         @param session_data: mutable data to use, default to a dict
         @param session_id (str): force the session_id to the given string
         @param profile: if set, the session is owned by the profile,
@@ -65,8 +68,7 @@
         if session_id is None:
             session_id = str(uuid4())
         elif session_id in self._sessions:
-            self._sessions[session_id][0].cancel()
-            log.warning(u"Session [{id}] is going to be re-initialised".format(id=session_id))
+            raise exceptions.ConflictError(u"Session id {} is already used".format(session_id))
         timer = reactor.callLater(self.timeout, self._purgeSession, session_id)
         if session_data is None:
             session_data = {}
@@ -74,8 +76,18 @@
         return session_id, session_data
 
     def _purgeSession(self, session_id):
+        try:
+            timer, session_data, profile = self._sessions[session_id]
+        except ValueError:
+            timer, session_data = self._sessions[session_id]
+            profile = None
+        try:
+            timer.cancel()
+        except error.AlreadyCalled:
+            # if the session is time-outed, the timer has been called
+            pass
         del self._sessions[session_id]
-        log.debug(u"Session [%s] purged" % session_id)
+        log.debug(u"Session {} purged{}".format(session_id, u' (profile {})'.format(profile) if profile is not None else u''))
 
     def __len__(self):
         return len(self._sessions)
@@ -88,6 +100,8 @@
             timer, session_data, profile_set = self._sessions[session_id]
         except ValueError:
             raise exceptions.InternalError("You need to use __getitem__ when profile is not set")
+        except KeyError:
+            raise failure.Failure(KeyError(MSG_NO_SESSION))
         if profile_set != profile:
             raise exceptions.InternalError("current profile differ from set profile !")
         if self.resettable_timeout:
@@ -99,6 +113,8 @@
             timer, session_data = self._sessions[session_id]
         except ValueError:
             raise exceptions.InternalError("You need to use profileGet instead of __getitem__ when profile is set")
+        except KeyError:
+            raise failure.Failure(KeyError(MSG_NO_SESSION))
         if self.resettable_timeout:
             timer.reset(self.timeout)
         return session_data
@@ -108,12 +124,9 @@
 
     def __delitem__(self, session_id):
         """ Cancel the timer, then actually delete the session data """
-        try:
-            timer = self._sessions[session_id][0]
-            timer.cancel()
-            self._purgeSession(session_id)
-        except KeyError:
-            log.debug(u"Session [%s] doesn't exists, timeout expired?" % session_id)
+        timer = self._sessions[session_id][0]
+        timer.cancel()
+        self._purgeSession(session_id)
 
     def keys(self):
         return self._sessions.keys()
@@ -134,7 +147,7 @@
         @return: a list containing the sessions ids
         """
         ret = []
-        for session_id in self._sessions:
+        for session_id in self._sessions.iterkeys():
             try:
                 timer, session_data, profile_set = self._sessions[session_id]
             except ValueError: