paho.mqtt.golang icon indicating copy to clipboard operation
paho.mqtt.golang copied to clipboard

invalid memory address or nil pointer dereference

Open zwzy29 opened this issue 3 years ago • 8 comments

panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x50c278]

goroutine 1052658 [running]:
io.ReadAtLeast(0x0, 0x0, 0xc00060b5b0, 0x1, 0x1, 0x1, 0xc00010f3e0, 0xc000546f78, 0x175c601)
	/usr/local/go/src/io/io.go:328 +0x58
io.ReadFull(...)
	/usr/local/go/src/io/io.go:347
meihuan.com/badger/vendor/github.com/eclipse/paho.mqtt.golang/packets.ReadPacket(0x0, 0x0, 0xc000546f00, 0x1000000010000, 0x4ac416, 0x10ec9a8)
	/home/build/go/src/meihuan.com/badger/vendor/github.com/eclipse/paho.mqtt.golang/packets/packets.go:115 +0xa5
meihuan.com/badger/vendor/github.com/eclipse/paho%2emqtt%2egolang.startIncoming.func1(0x0, 0x0, 0xc0002f40d0, 0xc0002f40c0, 0xc00010f860)
	/home/build/go/src/meihuan.com/badger/vendor/github.com/eclipse/paho.mqtt.golang/net.go:119 +0xfd
created by meihuan.com/badger/vendor/github.com/eclipse/paho%2emqtt%2egolang.startIncoming
	/home/build/go/src/meihuan.com/badger/vendor/github.com/eclipse/paho.mqtt.golang/net.go:117 +0x131

my env: go version:1.16.2 github.com/eclipse/paho.mqtt.golang v1.3.5

zwzy29 avatar Aug 31 '21 07:08 zwzy29

Are you sure you are using version 1.3.5? (I note that the library is vendored in your project so the version in use may not match your go.mod). The line numbers in the dump don't make sense with 1.3.5.

For example packets/packets.go:115 is part of the initialisation of a static variable. I could understand this error if you are using v1.1.1.

If you are using the latest version please provide more information (when this issue occurs, logging etc).

MattBrittan avatar Aug 31 '21 20:08 MattBrittan

I'm sure i am using version 1.3.5. Please checking this issue. Thank you

------------------ 原始邮件 ------------------ 发件人: "eclipse/paho.mqtt.golang" @.>; 发送时间: 2021年9月1日(星期三) 凌晨4:08 @.>; @.@.>; 主题: Re: [eclipse/paho.mqtt.golang] invalid memory address or nil pointer dereference (#529)

Are you sure you are using version 1.3.5? (I note that the library is vendored in your project so the version in use may not match your go.mod). The line numbers in the dump don't make sense with 1.3.5.

For example packets/packets.go:115 is part of the initialisation of a static variable. I could understand this error if you are using v1.1.1.

If you are using the latest version please provide more information (when this issue occurs, logging etc).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

zwzy29 avatar Sep 01 '21 07:09 zwzy29

Apologies - I was looking at master and changes to the copyright notices have pushed everything around (thought I checked the right tag this morning but was mistaken). So looking at the info in the crashlog:

io.go:328 nn, err = r.Read(buf[n:]) io.go:347 return ReadAtLeast(r, buf, len(buf)) packets/packets.go:115 - _, err := io.ReadFull(r, b) net.go:119 - if cp, err = packets.ReadPacket(conn); err != nil {

If appears that io.ReadFull is being called with a nil argument however the stack trace does not provide any info on potential causes. My assumption would be that this occurs on the initial connection meaning attemptConnection is returning a nil connection but no error; looking at that function I cannot see how that can happen.

Unfortunately without more information I'd be guessing at the cause and unlikely to find it. Please see the Reporting Bugs section of the readme for suggestions on the kinds of information that would help (at an absolute minimum I need a description of what is happening and to know what options you are using when setting up the client; however debug logs would make the process a lot easier).

MattBrittan avatar Sep 01 '21 08:09 MattBrittan

When the remote MQTT broker closed the connection, and i tried to reconnect for many times, then this panic occured. But i tried to stop my local MQTT broker later, this panic does not occur. The following are some of my code: var messagePubHandler mqtt.MessageHandler = func(client mqtt.Client, msg mqtt.Message) { log.Info().Msgf("Published message: %s from topic: %s\n", msg.Payload(), msg.Topic()) }

var connectHandler mqtt.OnConnectHandler = func(client mqtt.Client) { log.Info().Msg("Connected to MQTT broker") }

var connectLostHandler mqtt.ConnectionLostHandler = func(client mqtt.Client, err error) { log.Error().Msgf("Connect lost: %v", err) wait := 1 for { time.Sleep(time.Duration(wait) * time.Second) if token := client.Connect(); token.Wait() && token.Error() != nil { log.Info().Msg("Failed to reconnect, retry...") if wait < 32 { wait *= 2 } continue } break } }

func CreateMQTTClient(device *model.Device) (mqtt.Client, error) { opts := mqtt.NewClientOptions() opts.AddBroker(config.MQTT.Broker) opts.SetClientID(device.DeviceName) opts.SetUsername(device.Username) opts.SetPassword(device.Password) opts.SetDefaultPublishHandler(messagePubHandler) opts.OnConnect = connectHandler opts.OnConnectionLost = connectLostHandler client := mqtt.NewClient(opts) if token := client.Connect(); token.Wait() && token.Error() != nil { return nil, token.Error() } return client, nil }

I guess this maybe the reason for this panic: client.go:665 c.conn = nil      // Important that this is the only place that this is set to nil ------------------ 原始邮件 ------------------ 发件人: "eclipse/paho.mqtt.golang" @.>; 发送时间: 2021年9月1日(星期三) 下午4:28 @.>; @.@.>; 主题: Re: [eclipse/paho.mqtt.golang] invalid memory address or nil pointer dereference (#529)

Apologies - I was looking at master and changes to the copyright notices have pushed everything around (thought I checked the right tag this morning but was mistaken). So looking at the info in the crashlog:

io.go:328 nn, err = r.Read(buf[n:]) io.go:347 return ReadAtLeast(r, buf, len(buf)) packets/packets.go:115 - _, err := io.ReadFull(r, b) net.go:119 - if cp, err = packets.ReadPacket(conn); err != nil {

If appears that io.ReadFull is being called with a nil argument however the stack trace does not provide any info on potential causes. My assumption would be that this occurs on the initial connection meaning attemptConnection is returning a nil connection but no error; looking at that function I cannot see how that can happen.

Unfortunately without more information I'd be guessing at the cause and unlikely to find it. Please see the Reporting Bugs section of the readme for suggestions on the kinds of information that would help (at an absolute minimum I need a description of what is happening and to know what options you are using when setting up the client; however debug logs would make the process a lot easier).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

zwzy29 avatar Sep 01 '21 09:09 zwzy29

OK - that explains it.

By default this client automatically reconnects if the connection is lost (see the documentation). This means that you may well be calling Connect when another connection attempt is in progress; this is not something we expect a user to do so there are limited checks for this (the checks are not fully thread safe and may allow two attempts to proceed at the same time under some circumstances). Having two connections running at the same time could result in an issue like this (there will be a range of potential race conditions).

I'd suggest that you use the built in reconnection functionality (quite a bit of thought has gone into making this resilient) rather than doing this yourself. If you want to do it yourself turn the built in functionality off with SetAutoReconnect(false) and call Disconnect before attempting to reconnect (to ensure the library has fully shutdown the initial connection).

MattBrittan avatar Sep 01 '21 10:09 MattBrittan

OK, thank you for your reply

------------------ 原始邮件 ------------------ 发件人: "eclipse/paho.mqtt.golang" @.>; 发送时间: 2021年9月1日(星期三) 晚上6:16 @.>; @.@.>; 主题: Re: [eclipse/paho.mqtt.golang] invalid memory address or nil pointer dereference (#529)

OK - that explains it.

By default this client automatically reconnects if the connection is lost (see the documentation). This means that you may well be calling Connect when another connection attempt is in progress; this is not something we expect a user to do so there is no code to stop you. Having two connections running at the same time could result in an issue like this (there will be a range of race conditions).

I'd suggest that you use the built in reconnection functionality (quite a bit of thought has gone into making this resilient) rather than doing this yourself. If you want to do it yourself turn the build in functionality off with SetAutoReconnect(false) and call Disconnect before attempting to reconnect (to ensure the library has fully shutdown the initial connection).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

zwzy29 avatar Sep 01 '21 10:09 zwzy29

any docker update for this issues?

ahsandev2019 avatar Sep 04 '21 21:09 ahsandev2019

@ahsandev2019 - sorry I'm not sure what you mean by your comment.

This particular issue is due to the library being used in an unexpected manner (using the automatic reconnect while also calling connect when the connection drops). While it would be possible to remove the potential race condition in Connect I don't think its a high priority as I would not expect it to be an issue other than when using the library incorrectly (as was the case here). This has nothing to do with docker so I'm a bit confused - if you have a problem perhaps raise a new issue.

MattBrittan avatar Sep 05 '21 08:09 MattBrittan

I'm going to close this because I believe it was addressed in PR #607 (the connection status is now actively monitored).

MattBrittan avatar Jan 06 '24 03:01 MattBrittan