Changeset 99:2c8dc93fbef4 for wokkel

Timestamp:

Mar 7, 2011, 7:40:16 PM (11 years ago)

Author:

Ralph Meijer <ralphm@…>

Branch:

default

Message:

Fix race condition and nesting errors when adding a new subprotocol handler.

This fixes two related issues with adding a new subprotocol handler:

• Adding a handler when the stream is not yet initialized (authenticated) does not cause connectionMade to be called.
• Adding a handler in connectionMade, connectionInitialized, or connectionLost modifies the lists of handlers iterated over, causing some methods being called too often.

Author: ff, kandaurovoleg, ralphm.
Fixes: #48.

Location:

wokkel

Files:

• subprotocols.py (modified) (4 diffs)
• test/test_subprotocols.py (modified) (2 diffs)

wokkel/subprotocols.py

@@ -186 +186 @@
         # get protocol handler up to speed when a connection has already
         # been established
-        if self.xmlstream and self._initialized:
+        if self.xmlstream:
             handler.makeConnection(self.xmlstream)
+        if self._initialized:
             handler.connectionInitialized()
 
@@ -211 +212 @@
         self.xmlstream = xs
 
-        for e in self:
+        for e in list(self):
             e.makeConnection(xs)
 
@@ -230 +231 @@
         # Notify all child services which implement
         # the IService interface
-        for e in self:
+        for e in list(self):
             e.connectionInitialized()
 
@@ -265 +266 @@
         # Notify all child services which implement
         # the IService interface
-        for e in self:
+        for e in list(self):
             e.connectionLost(reason)
 

wokkel/test/test_subprotocols.py

@@ -287 +287 @@
 
 
+    def test_addHandlerConnected(self):
+        """
+        Adding a handler when connected doesn't call connectionInitialized.
+        """
+        sm = self.streamManager
+        xs = xmlstream.XmlStream(xmlstream.Authenticator())
+        sm._connected(xs)
+        handler = DummyXMPPHandler()
+        handler.setHandlerParent(sm)
+
+        self.assertEquals(1, handler.doneMade)
+        self.assertEquals(0, handler.doneInitialized)
+        self.assertEquals(0, handler.doneLost)
+
+
+    def test_addHandlerConnectedNested(self):
+        """
+        Adding a handler in connectionMade doesn't cause 2nd call.
+        """
+        class NestingHandler(DummyXMPPHandler):
+            nestedHandler = None
+
+            def connectionMade(self):
+                DummyXMPPHandler.connectionMade(self)
+                self.nestedHandler = DummyXMPPHandler()
+                self.nestedHandler.setHandlerParent(self.parent)
+
+        sm = self.streamManager
+        xs = xmlstream.XmlStream(xmlstream.Authenticator())
+        handler = NestingHandler()
+        handler.setHandlerParent(sm)
+        sm._connected(xs)
+
+        self.assertEquals(1, handler.doneMade)
+        self.assertEquals(0, handler.doneInitialized)
+        self.assertEquals(0, handler.doneLost)
+
+        self.assertEquals(1, handler.nestedHandler.doneMade)
+        self.assertEquals(0, handler.nestedHandler.doneInitialized)
+        self.assertEquals(0, handler.nestedHandler.doneLost)
+
+
+
     def test_addHandlerInitialized(self):
         """
@@ -306 +349 @@
         self.assertEquals(1, handler.doneInitialized)
         self.assertEquals(0, handler.doneLost)
+
+
+    def test_addHandlerInitializedNested(self):
+        """
+        Adding a handler in connectionInitialized doesn't cause 2nd call.
+        """
+        class NestingHandler(DummyXMPPHandler):
+            nestedHandler = None
+
+            def connectionInitialized(self):
+                DummyXMPPHandler.connectionInitialized(self)
+                self.nestedHandler = DummyXMPPHandler()
+                self.nestedHandler.setHandlerParent(self.parent)
+
+        sm = self.streamManager
+        xs = xmlstream.XmlStream(xmlstream.Authenticator())
+        handler = NestingHandler()
+        handler.setHandlerParent(sm)
+        sm._connected(xs)
+        sm._authd(xs)
+
+        self.assertEquals(1, handler.doneMade)
+        self.assertEquals(1, handler.doneInitialized)
+        self.assertEquals(0, handler.doneLost)
+
+        self.assertEquals(1, handler.nestedHandler.doneMade)
+        self.assertEquals(1, handler.nestedHandler.doneInitialized)
+        self.assertEquals(0, handler.nestedHandler.doneLost)
+
+
+    def test_addHandlerConnectionLostNested(self):
+        """
+        Adding a handler in connectionLost doesn't call connectionLost there.
+        """
+        class NestingHandler(DummyXMPPHandler):
+            nestedHandler = None
+
+            def connectionLost(self, reason):
+                DummyXMPPHandler.connectionLost(self, reason)
+                self.nestedHandler = DummyXMPPHandler()
+                self.nestedHandler.setHandlerParent(self.parent)
+
+        sm = self.streamManager
+        xs = xmlstream.XmlStream(xmlstream.Authenticator())
+        handler = NestingHandler()
+        handler.setHandlerParent(sm)
+        sm._connected(xs)
+        sm._authd(xs)
+        sm._disconnected(xs)
+
+        self.assertEquals(1, handler.doneMade)
+        self.assertEquals(1, handler.doneInitialized)
+        self.assertEquals(1, handler.doneLost)
+
+        self.assertEquals(0, handler.nestedHandler.doneMade)
+        self.assertEquals(0, handler.nestedHandler.doneInitialized)
+        self.assertEquals(0, handler.nestedHandler.doneLost)
+
 
     def test_removeHandler(self):