Code review comment for lp:~free.ekanayaka/landscape-client/socket-locks-fix

Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

[1]

Wow, that was a nice analysis. Thanks for all the hints it was quite interesting, changed as you suggest and also added:

   find "${SOCKETDIR:?}" -type l -print0 | xargs -r0 rm -f

because we need to delete the symbolic links that Twisted uses to keep track of the PID that created the socket.

[2]

Yeah I know, the whole TwistedReactorTest is somehow unfortunate (actually I've been wondering if we need a TwistedReactor class at all and if we could in principle go with the straight Twisted reactor). Those tests have been there for a long time and they seem to work (read "pass"), so I don't think we need to touch them for now, or at least not in this branch.

As for patching the global reactor, good catch :) I've added an addCleanup:

        saved_stop = reactor._reactor.stop
        reactor._reactor.stop = reactor._reactor.crash
        self.addCleanup(lambda: setattr(reactor._reactor, "stop", saved_stop))
        return reactor

which I believe fixes the issue.

« Back to merge proposal