Heartbeat._bind_socket continually retries on success

[mdickinson]

I think there's a logic bug in the implementation of Heartbeat._bind_socket. The current code looks like this:

        max_attempts = 1 if self.original_port else 100
        for attempt in range(max_attempts):
            try:
                self._try_bind_socket()
            except zmq.ZMQError as ze:
                if attempt == max_attempts - 1:
                    raise
                # Raise if we have any error not related to socket binding
                if ze.errno != errno.EADDRINUSE and ze.errno != win_in_use:
                    raise
                # Raise if we have any error not related to socket binding
                if self.original_port == 0:
                    self.pick_port()
                else:
                    raise

If self._try_bind_socket fails with an exception, we try again; that part makes sense to me. But if self._try_bind_socket succeeds ..., we also enter the next iteration of the for loop and try again. I think there should be a return or a break at that point.

When I test this on my machine (in the case where self.original_port is False), the effect is that 50 different ports are tried, twice each: the first attempt of each pair succeeds, and the second one fails (because the port is already in use). The end result is that the run method is abandoned and the heartbeat isn't ever properly started.

Changing 100 to 101 (or 99) allows the heartbeat to start. That's obviously not the right fix. :-)

[mdickinson]

Fix in #431, but I just noticed that @pauldmccarthy also provided a fix in #429, and that fix also looks good to me. FWIW, the #431 fix has a regression test. Not sure what the easiest way forward is here: I can patch that regression test into #429 if that makes sense.

[pauldmccarthy]

I'm also happy for #429 to be closed - given that you've added a test, yours is better :)