reconnect() no longer disconnects already connected socket

[vladak]

In the past reconnect() disconnected on TCP level first and then created new connection to the broker. This is no longer the case and some MQTT broker implementations will (correctly, see below) refuse such reconnect attempts due to duplicate client ID. Previously it worked because _get_connect_socket() used by connect() did self._sock.close() so I assume this changed with the conversion to the connection manager.

The spec says in 3.1.4 Response:

If the ClientId represents a Client already connected to the Server then the Server MUST disconnect the existing Client [MQTT-3.1.4-2].

[vladak]

The fix should be accompanied by reconnect test, including the resubscribe capability.

[justmobilize]

@vladak do you know a server setup that will cause this to happen? Happy to fix this, but would like real world repro steps if possible

[vladak]

This happened with a custom from-scratch written implementation of a MQTT broker which actually followed the spec, haven't checked any mainline implementations yet. I have a fix in the works, the tests are a bit tricky.

[vladak]

Also, one interesting tidbit: when connect() is called with a particular session_id, subsequent reconnect() actually grabs new socket because it does not supply the original session ID and hence the connection manager does not match it.

With the change proposed in PR #244 this will no longer be a problem in terms of wasted socket, however the original session ID should be used for the reconnect.

[justmobilize]

@vladak this should be tested on an ESP32SPI. When I added session support to ConnectionManager, the ESP32SPI would have issues if it was already connected to a host