Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Publishing freeze on connection lost #325

Closed
panter-dsd opened this issue Jul 1, 2019 · 3 comments
Closed

Publishing freeze on connection lost #325

panter-dsd opened this issue Jul 1, 2019 · 3 comments

Comments

@panter-dsd
Copy link

panter-dsd commented Jul 1, 2019

Version: master
If the option options.AutoReconnect is false then options.MessageChannelDepth is set to 0 here. So, we have a channel with zero length. When we get into the line

c.obound <- &PacketAndToken{p: pub, t: token}
and connection is lost (channel reader has already exited), we're stuck at this line and never leave the Publish function.

panter-dsd added a commit to litmusautomation/paho.mqtt.golang that referenced this issue Jul 1, 2019
panter-dsd added a commit to litmusautomation/paho.mqtt.golang that referenced this issue Jul 3, 2019
eclipse#325

Signed-off-by: Konstantin Konnov <konstantin.konnov@litmusautomation.com>
@ChIoT-Tech
Copy link

I believe that the fix introduces a new issue; if WriteTimeout is set to 0 (the default) then the "case <-time.After(c.options.WriteTimeout):" triggers immediately. This sometimes (approx 10% in my test code) ends up getting selected before the write to c.obound meaning that the publish message token is flagged as being in error state incorrectly (and the published message is lost despite AutoReconenct and persistence being active).

While setting WriteTimeout to a non-zero value would fix this the default value is currently is 0 (supposed to mean no timeout) and when used with AutoReconenct and Persistence these values make sense as the data will be written at some point.

The simplest solution might be:

if c.options.WriteTimeout == 0 {
	c.obound <- &PacketAndToken{p: pub, t: token}
} else {
	select {
	case c.obound <- &PacketAndToken{p: pub, t: token}:
	case <-time.After(c.options.WriteTimeout):
		token.setError(errors.New("publish was broken by timeout"))
	}
}

panter-dsd added a commit to litmusautomation/paho.mqtt.golang that referenced this issue Jul 13, 2019
eclipse#325

Signed-off-by: Konstantin Konnov <konstantin.konnov@litmusautomation.com>
@panter-dsd
Copy link
Author

@ChIoT-Tech sorry, I didn't notice this. Fixed. Better would be to using contexts but this will change API.

alekseytols90 pushed a commit to alekseytols90/pahogolangwork that referenced this issue Mar 7, 2020
eclipse/paho.mqtt.golang#325

Signed-off-by: Konstantin Konnov <konstantin.konnov@litmusautomation.com>
alekseytols90 pushed a commit to alekseytols90/pahogolangwork that referenced this issue Mar 7, 2020
eclipse/paho.mqtt.golang#325

Signed-off-by: Konstantin Konnov <konstantin.konnov@litmusautomation.com>
@MattBrittan
Copy link
Contributor

Closing this because the change has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants