source: ralphm-patches/addHandler-48.patch @ 47:f6d222b68f1c

# HG changeset patch
# Parent bcf4497abc14816955c62dc9429a8d61f0cfe703
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.

diff -r bcf4497abc14 wokkel/subprotocols.py
--- a/wokkel/subprotocols.py    Mon Feb 28 20:40:04 2011 +0100
+++ b/wokkel/subprotocols.py    Mon Feb 28 20:53:55 2011 +0100
@@ -185,8 +185,9 @@
 
         # 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()
 
 
@@ -210,7 +211,7 @@
 
         self.xmlstream = xs
 
-        for e in self:
+        for e in list(self):
             e.makeConnection(xs)
 
 
@@ -229,7 +230,7 @@
 
         # Notify all child services which implement
         # the IService interface
-        for e in self:
+        for e in list(self):
             e.connectionInitialized()
 
 
@@ -264,7 +265,7 @@
 
         # Notify all child services which implement
         # the IService interface
-        for e in self:
+        for e in list(self):
             e.connectionLost(reason)
 
 
diff -r bcf4497abc14 wokkel/test/test_subprotocols.py
--- a/wokkel/test/test_subprotocols.py  Mon Feb 28 20:40:04 2011 +0100
+++ b/wokkel/test/test_subprotocols.py  Mon Feb 28 20:53:55 2011 +0100
@@ -286,6 +286,49 @@
         self.assertEquals(0, handler.doneLost)
 
 
+    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):
         """
         Test the addition of a protocol handler after the stream
@@ -306,6 +349,64 @@
         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):
         """
         Test removal of protocol handler.