Should this library still use x/sync/semaphore?

[rittneje]

The v5 library has a comment explaining why it no longer uses x/sync/semaphore.

// This function was previously performed by `golang.org/x/sync/semaphore` but, as per the MQTT spec:
//
// > The send quota is not incremented if it is already equal to the initial send quota. The attempt to increment above
// > the initial send quota might be caused by the re-transmission of a PUBREL packet after a new Network Connection is
// > established.
//
// The result of this happening with `semaphore` is a `panic` which is not ideal.
// It is also possible (as per issue #179) that bugs, or unexpected circumstances, may result in the same situation. For
// example: if the local session state is lost but there is a session state on the server (meaning it sends an unexpected
// PUBACK).

Does this problem also apply to this library? Should it not be using x/sync/semaphore?

[MattBrittan]

The V5 spec mandates how quotas work meaning that the client needs to follow these rules. The V3 spec dows not define this so the implementation here is totally within our control (and much more limited than in the V5 client).

I don't believe there is an issue in this client because it only uses a semaphore to limit the number of messages in flight when sending messages in the store following reconnection (MaxResumePubInFlight). It does not provide the option to limit messages in flight in general. MaxResumePubInFlight was added because it's possible for a lot of messages to be added to the session state while the client is disconnected, and attempting to send them all quickly following reconnection can cause issues (particularly over mobile connections).