eclipse / paho.golang

Go libraries
Other
327 stars 92 forks source link

Improve default ping handler #257

Closed Lorderot closed 3 months ago

Lorderot commented 3 months ago

Default PingHandler

Current state

In order to detect network issues in the application layer, PingHandler sends a PINGREQ, and if it doesn't receive a response within the specified KeepAlive period, the connection is closed. However, if the network is very slow and unstable, the WriteTo(conn) operation (which sends a PINGREQ) may block for quite some time, during which context cancellation and KeepAlive timeout are ignored. The call must be eventually unblocked due to TCP timeouts. (Note: I have an example where connection establishment through the proxy hung for 1 hour; it's already fixed with proxy.Dial and ContextDialer. However, I suspect the same could happen here as well)

Proposal

Send a PINGREQ request in a separate goroutine. In case of KeepAlive timeout or context cancellation, the main goroutine does not wait until the write operation finishes. The spawned goroutine will not be leaked if the connection is closed (this must unblock the write operation).