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

Panic on connection lost / reconnecting #536

Closed
tvojacek opened this issue Sep 16, 2021 · 2 comments
Closed

Panic on connection lost / reconnecting #536

tvojacek opened this issue Sep 16, 2021 · 2 comments

Comments

@tvojacek
Copy link

I experience similar problem as #488

paho v1.3.5
ClientOptions{
ClientID:"devel-02",
CleanSession:false
Order:false
KeepAlive:2
AutoReconnect:true
MaxReconnectInterval:2s
}

running approx 40 client in parallel

simplified section of code causing panic:

for{
  if !client.IsConnected(){
    client.Disconnect(10)
    client.Connect()
  }
  time.Sleep(100*time.Millisecond())
}

error message:
panic: close of closed channel

goroutine 8466 [running]:
github.com/eclipse/paho%2emqtt%2egolang.(*client).startCommsWorkers.func2(0xc00a3604b8, 0xc00a3604c0, 0xc00a30be00, 0xc0018a8b40)
/go/pkg/mod/github.com/eclipse/[email protected]/client.go:637 +0x53c
created by github.com/eclipse/paho%2emqtt%2egolang.(*client).startCommsWorkers
/go/pkg/mod/github.com/eclipse/[email protected]/client.go:597 +0x4b3

@MattBrittan
Copy link
Contributor

Hi @TomasVojacek,

I was aware that there was an issue here (there are a few other similar issues) - reusing the client after calling Disconnect is not fully threadsafe (for example if a connection attempt is in progress Disconnect may return before it exits leading to this panic). However as I currently have limited time and cannot see a use-case for this (the built in reconnection functionality should handle reconnections reliably for you) its not something I will be looking at in the short term. For the time being I have added a warning to the Disconnect function so that users are alerted to the issue (and their options).

Would the built in functionality (see SetAutoReconnect and SetConnectRetry) work for you?

Happy to accept a pull request fixing this. However I don't think it will be a simple fix; due to the way this library has evolved its difficult to maintain thread safety (really easy to introduce unintended deadlocks) and the way client.status is used is somewhat problematic (for historical reasons).

Matt

@MattBrittan
Copy link
Contributor

I'm going to close this as a duplicate of #550 (realise that this issue was raised earlier but that issue more clearly states the issue).

Hopefully the additional comments in the readme and Disconnect function prevent others from running into this issue.

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