search

eclipse / paho.mqtt.golang

Other
2.73k stars 533 forks source link

remove unused error errAlreadyHandlingConnectionLoss #672

Closed wcsiu closed 3 months ago

wcsiu commented 5 months ago

Chanages

MattBrittan commented 5 months ago

Sorry - I will be unable to accept this without a signed ECA (see the readme and the above link for more info).

The important bit of this PR would be || c.status == disconnected; however connectionStatus was designed such that transitioning from connecting to disconnected should not happen (if it does then there are potential race conditions). Consider the situation where the user calls Disconnect and then Connect again before connected gets called (seems possible from a quick review of the code).

However I believe that connectionStatus already accounts for this. Disconnecting() waits on c.actionCompleted before completing if the status was connecting or reconnecting (which should prevent the status from becoming disconnected before connected is called.

Now it is possible that I'm missing something (wrote this code a while back, it's quite complex, and I've just scanned it). Are you able to replicate the issue you are attempting to address?

wcsiu commented 5 months ago

Sorry - I will be unable to accept this without a signed ECA (see the readme and the above link for more info).

The important bit of this PR would be || c.status == disconnected; however connectionStatus was designed such that transitioning from connecting to disconnected should not happen (if it does then there are potential race conditions). Consider the situation where the user calls Disconnect and then Connect again before connected gets called (seems possible from a quick review of the code).

However I believe that connectionStatus already accounts for this. Disconnecting() waits on c.actionCompleted before completing if the status was connecting or reconnecting (which should prevent the status from becoming disconnected before connected is called.

Now it is possible that I'm missing something (wrote this code a while back, it's quite complex, and I've just scanned it). Are you able to replicate the issue you are attempting to address?

I think you are right about it that it will never reach disconnected state for that conditional check. My mistake on that.

MattBrittan commented 3 months ago

Closing this as I cannot accept it without an ECA (see here for more details). Realise it's only a very small change but the rules are the rules (and are there to protect the integrity of the project). If you are able to sort out the ECA then this can be reopened.