r46779 - fix the bug

Author: glyph
Date: Fri Feb 12 04:12:42 2016
New Revision: 46779

Modified:
   branches/hostname-endpoint-8014/twisted/internet/endpoints.py

Log:
fix the bug

also refactor somewhat for comprehensibility

Modified: branches/hostname-endpoint-8014/twisted/internet/endpoints.py
==============================================================================
--- branches/hostname-endpoint-8014/twisted/internet/endpoints.py	(original)
+++ branches/hostname-endpoint-8014/twisted/internet/endpoints.py	Fri Feb 12 04:12:42 2016
@@ -669,35 +669,14 @@
         connection which is established first.
         """
         wf = protocolFactory
-        pending = []
-
-        def _canceller(d):
-            """
-            The outgoing connection attempt was cancelled.  Fail that L{Deferred}
-            with an L{error.ConnectingCancelledError}.
-
-            @param d: The L{Deferred <defer.Deferred>} that was cancelled
-            @type d: L{Deferred <defer.Deferred>}
-
-            @return: C{None}
-            """
-            d.errback(error.ConnectingCancelledError(
-                HostnameAddress(self._host, self._port)))
-            for p in pending[:]:
-                p.cancel()
-
-        def errbackForGai(failure):
-            """
-            Errback for when L{_nameResolution} returns a Deferred that fires
-            with failure.
-            """
-            return defer.fail(error.DNSLookupError(
-                "Couldn't find the hostname '%s'" % (self._host,)))
-
-        def _endpoints(gaiResult):
+        d = self._nameResolution(self._host, self._port)
+        d.addErrback(lambda ignored: defer.fail(error.DNSLookupError(
+            "Couldn't find the hostname '%s'" % (self._host,))))
+        @d.addCallback
+        def gaiResultToEndpoints(gaiResult):
             """
             This method matches the host address family with an endpoint for
-            every address returned by GAI.
+            every address returned by C{getaddrinfo}.
 
             @param gaiResult: A list of 5-tuples as returned by GAI.
             @type gaiResult: list
@@ -709,72 +688,79 @@
                 elif family in [AF_INET]:
                     yield TCP4ClientEndpoint(self._reactor, sockaddr[0],
                             sockaddr[1], self._timeout, self._bindAddress)
-                        # Yields an endpoint for every address returned by GAI
+                    # Yields an endpoint for every address returned by GAI
 
+        def _canceller(d):
+            # This canceller must remain defined outside of
+            # `attemptConnection`, because Defereds should not participate in
+            # cycles with their cancellers; that would create a potentially
+            # problematic circular reference and possibly gc.garbage.
+            d.errback(error.ConnectingCancelledError(
+                HostnameAddress(self._host, self._port)))
+
+        @d.addCallback
         def attemptConnection(endpoints):
             """
-            When L{endpoints} yields an endpoint, this method attempts to connect it.
+            When L{gaiResultToEndpoints} yields an endpoint, this function
+            attempts to connect it.  The trial attempts for each endpoints, the
+            recording of successful and failed attempts, and the algorithm to
+            pick the winner endpoint goes here.
+
+            @return: a Deferred that fires with the result of the
+                C{endpoint.connect} method that completes the fastest, or fails
+                with the first connection error it encountered if none of them
+                succeed.
             """
-            # The trial attempts for each endpoints, the recording of
-            # successful and failed attempts, and the algorithm to pick the
-            # winner endpoint goes here.
-            # Return a Deferred that fires with the endpoint that wins,
-            # or `failures` if none succeed.
-
-            endpointsListExhausted = []
-            successful = []
+            pending = []
             failures = []
             winner = defer.Deferred(canceller=_canceller)
 
-            def usedEndpointRemoval(connResult, connAttempt):
-                pending.remove(connAttempt)
-                return connResult
-
-            def afterConnectionAttempt(connResult):
-                if lc.running:
-                    lc.stop()
-
-                successful.append(True)
-                for p in pending[:]:
-                    p.cancel()
-                winner.callback(connResult)
-                return None
-
             def checkDone():
-                if endpointsListExhausted and not pending and not successful:
-                    winner.errback(failures.pop())
-
-            def connectFailed(reason):
-                failures.append(reason)
-                checkDone()
-                return None
+                if pending or checkDone.completed or checkDone.endpointsLeft:
+                    return
+                winner.errback(failures.pop())
+            checkDone.completed = False
+            checkDone.endpointsLeft = True
 
+            @LoopingCall
             def iterateEndpoint():
-                try:
-                    endpoint = next(endpoints)
-                except StopIteration:
+                endpoint = next(endpoints, None)
+                if endpoint is None:
                     # The list of endpoints ends.
-                    endpointsListExhausted.append(True)
-                    lc.stop()
+                    checkDone.endpointsLeft = False
+                    iterateEndpoint.stop()
+                    checkDone()
+                    return
+
+                eachAttempt = endpoint.connect(wf)
+                pending.append(eachAttempt)
+                @eachAttempt.addBoth
+                def noLongerPending(result):
+                    pending.remove(eachAttempt)
+                    return result
+                @eachAttempt.addCallback
+                def succeeded(result):
+                    if iterateEndpoint.running:
+                        iterateEndpoint.stop()
+                    winner.callback(result)
+                @eachAttempt.addErrback
+                def failed(reason):
+                    failures.append(reason)
                     checkDone()
-                else:
-                    dconn = endpoint.connect(wf)
-                    pending.append(dconn)
-                    dconn.addBoth(usedEndpointRemoval, dconn)
-                    dconn.addCallback(afterConnectionAttempt)
-                    dconn.addErrback(connectFailed)
-
-            lc = LoopingCall(iterateEndpoint)
-            lc.clock = self._reactor
-            lc.start(0.3)
+
+            iterateEndpoint.clock = self._reactor
+            iterateEndpoint.start(0.3)
+            @winner.addBoth
+            def cancelRemainingPending(result):
+                checkDone.completed = True
+                for remaining in pending[:]:
+                    remaining.cancel()
+                return result
             return winner
 
-        d = self._nameResolution(self._host, self._port)
-        d.addErrback(errbackForGai)
-        d.addCallback(_endpoints)
-        d.addCallback(attemptConnection)
         return d
 
+
     def _nameResolution(self, host, port):
         """
         Resolve the hostname string into a tuple containig the host