Opened 9 years ago

Closed 6 years ago

#1128 enhancement closed wontfix (wontfix)

Protocol-level wrapper for transport.loseConnection

Reported by: subterrific Owned by:
Priority: low Milestone:
Component: core Keywords:
Cc: exarkun, subterrific Branch:
Author: Launchpad Bug:

Description (last modified by glyph)

Protocol should have a wrapper around transport.loseConnection that returns a deferred which is fired when connectionLost() is called.

Change History (6)

comment:1 Changed 9 years ago by subterrific

Protocol should have a wrapper around transport.loseConnection that returns a
deferred which is fired when connectionLost() is called.

comment:2 Changed 8 years ago by exarkun

  • Component set to core

comment:3 Changed 7 years ago by exarkun

  • Priority changed from normal to low

Still not a compelling use-case for me. A mixin which provides this might be handy, but I doubt I'll write it. If someone else wants to, please do.

comment:4 Changed 6 years ago by ghazel

Oops, wrote this up in #3587 then found this ticket. Here's a compelling use case:


class MonarchyProtocol(Protocol):

    def connectionMade(self):
        self.factory.appointOneTrueKing(self)

    def connectionLost(self, reason):
        self.factory.abdicateOneTrueKing(self)


class MonarchyFactory(Factory):

    protocol = MonarchyProtocol

    def __init__(self):
        self.monarch = None

    def appointOneTrueKing(self, monarch):
        if self.monarch:
            df = self.monarch.transport.loseConnection()
            if df:
                f = lambda r: self.appointOneTrueKing(monarch)
                df.addBoth(f)
                return
        assert self.monarch is None
        self.monarch = monarch

    def abdicateOneTrueKing(self, monarch):
        assert self.monarch == monarch
        self.monarch = None

comment:5 Changed 6 years ago by glyph

  • Description modified (diff)
  • Resolution set to wontfix
  • Status changed from new to closed

I find myself strangely ... uncompelled. First off, this code is broken. Unless loseConnection always fires synchronously (and if it does, why do we need this) then self.monarch won't always be None when it hits that assert in appointOneTrueKing. I can't actually tell what you want here - if connection 1 arrives, then connection 2 arrives, and 1 disconnects, but connection 3 arrives before connection 1 finishes going away: who wins, 2 or 3? (Assuming you agree with the proposed placement of this deferred, you also don't upcall to connectionLost; how do you expect Protocol to get notified of the connection being dropped to fire its Deferred.)

Second, this implicitly depends on a potentially confusing required ordering of connectionLost and the Deferred firing.

Third, why not just write it like this?

class MonarchyProtocol(Protocol):
    def connectionMade(self):
        self._cldeferred = Deferred()
        self.factory.appoint(self)
    def abdicate(self):
        result = Deferred()
        self._cldeferred.chainDeferred(result)
        return result
    def connectionLost(self, reason):
        if self.factory.monarch is self:
            self.factory.monarch = None
        self._cldeferred.callback(None)
class MonarchyFactory(Factory):
    protocol = MonarchyProtocol
    def __init__(self):
        self.monarch = None
    def appoint(self, newKing):
        if self.monarch is not None:
            self.monarch.abdicate().addCallback(self.appoint, newKing)
            return
        self.monarch = newKing

This is exactly as long as your example, but I think it's a bit easier to read (in addition to fixing the aforementioned bugs).

Unless somebody wants to provide a very clear justification for this, I am -1 on adding this. I think it's going to be complex and tricky to implement, and I think that it's going to be an attractor for bugs in application code which assume an ordering of events related to this deferred's callback that may not be guaranteed. (I'm also updating the description for posterity.)

comment:6 Changed 3 years ago by <automation>

  • Owner exarkun deleted
Note: See TracTickets for help on using tickets.