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 when Disconnect() invoked after internalConnLost #19

Closed
jpwsutton opened this issue Feb 9, 2016 · 1 comment
Closed

panic when Disconnect() invoked after internalConnLost #19

jpwsutton opened this issue Feb 9, 2016 · 1 comment
Labels

Comments

@jpwsutton
Copy link
Member

migrated from Bugzilla #464559
status RESOLVED severity normal in component MQTT-Go for 1.1
Reported in version 0.9 on platform PC
Assigned to: Al Stockdill-Mander

On 2015-04-14 00:22:00 -0400, shirou WAKAYAMA wrote:

paho library causes panic when invoke Disconnect().

Here is my sample program.

package main

import (
"fmt"
"time"

MQTT "git.eclipse.org/gitroot/paho/org.eclipse.paho.mqtt.golang.git"
)

func main() {
opts := MQTT.NewClientOptions().AddBroker("tcp://localhost:1883")
opts.SetCleanSession(true)

c := MQTT.NewClient(opts)
if token := c.Connect(); token.Wait() && token.Error() != nil {
panic(token.Error())
}
fmt.Println("plz down mosquitto now")
time.Sleep(5 * time.Second)

c.Disconnect(200)
time.Sleep(5 * time.Second)

}

How to reproduce:

  1. mosquitto up
  2. go run close_panic.go
  3. mosquitto down after "plz down mosquitto now" shows
  4. wait 5 sec
  5. panic

This is caused by close(c.stop) in the Client.disconnect(). c.stop is already closed at the internalConnLost(), so the second disconnect() close already closed channel.

To avoid panic, I think check the channel is already close or not using select/case.

@@ -382,7 +383,12 @@ func (c *Client) internalConnLost(err error) {
}

func (c *Client) disconnect() {

  •   close(c.stop)
    
  •   select {
    
  •   case _, ok := <-c.stop:
    
  •           if ok {
    
  •                   close(c.stop)
    
  •           }
    
  •   }
    

It can fix problem at least my environment.

Thank you.

On 2015-04-14 00:42:15 -0400, shirou WAKAYAMA wrote:

I forget to add error and my environment.

go version go1.4.2 darwin/amd64

panic: close of closed channel

goroutine 1 [running]:
git.eclipse.org/gitroot/paho/org%2eeclipse%2epaho%2emqtt%2egolang%2egit.(_Client).disconnect(0xc20805e000)
/Users/blah/src/git.eclipse.org/gitroot/paho/org.eclipse.paho.mqtt.golang.git/client.go:385 +0x3a
git.eclipse.org/gitroot/paho/org%2eeclipse%2epaho%2emqtt%2egolang%2egit.(_Client).Disconnect(0xc20805e000, 0xc8)
/Users/blah/src/git.eclipse.org/gitroot/paho/org.eclipse.paho.mqtt.golang.git/client.go:355 +0x466
main.main()
/Users/blah/src/git.eclipse.org/gitroot/paho/org.eclipse.paho.mqtt.golang.git/samples/close_panic.go:21 +0x210

goroutine 17 [syscall, locked to thread]:
runtime.goexit()
/usr/local/Cellar/go/1.4.2/libexec/src/runtime/asm_amd64.s:2232 +0x1

goroutine 6 [select]:
git.eclipse.org/gitroot/paho/org%2eeclipse%2epaho%2emqtt%2egolang%2egit.func·003()
/Users/blah/src/git.eclipse.org/gitroot/paho/org.eclipse.paho.mqtt.golang.git/router.go:131 +0x4e6
created by git.eclipse.org/gitroot/paho/org%2eeclipse%2epaho%2emqtt%2egolang%2egit.(*router).matchAndDispatch
/Users/blah/src/git.eclipse.org/gitroot/paho/org.eclipse.paho.mqtt.golang.git/router.go:161 +0x166

goroutine 13 [sleep]:
git.eclipse.org/gitroot/paho/org%2eeclipse%2epaho%2emqtt%2egolang%2egit.(_Client).reconnect(0xc20805e000)
/Users/blah/src/git.eclipse.org/gitroot/paho/org.eclipse.paho.mqtt.golang.git/client.go:287 +0xd83
created by git.eclipse.org/gitroot/paho/org%2eeclipse%2epaho%2emqtt%2egolang%2egit.(_Client).internalConnLost
/Users/blah/src/git.eclipse.org/gitroot/paho/org.eclipse.paho.mqtt.golang.git/client.go:377 +0xe5

exit status 2

Thank you.

On 2015-04-17 11:19:32 -0400, Al Stockdill-Mander wrote:

http://git.eclipse.org/c/paho/org.eclipse.paho.mqtt.golang.git/commit/?id=SHA: 15034ff

Thanks

@jpwsutton jpwsutton added the bug label Feb 9, 2016
@dh1tw
Copy link

dh1tw commented Jul 31, 2016

I can confirm that this bug still exists in the latest master version (#44eec14).

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

No branches or pull requests

2 participants