SIGSEGV crash when running examples/tcp/main.go

[ogelami]

I noticed this issue when i switched from my PC to my raspberry pi, trying to narrow down the cause it seems like i get the same SIGSEGV error when running the tcp example.

Running on Raspberry Pi 3 Model B Plus Rev 1.3, ARMv7 Processor rev 4 (v7l). I've confirmed that there are no issues to bind to the port.

root@housekeeper:/home/ogelami/mochi-mqtt-issue# go run main.go

Mochi MQTT Server initializing... TCP
  Started!
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1238c]

goroutine 22 [running]:
runtime/internal/atomic.goLoad64(0x1cae03c, 0x0, 0x0)
        /usr/lib/go-1.15/src/runtime/internal/atomic/atomic_arm.go:131 +0x1c
github.com/mochi-co/mqtt/server/listeners.(*TCP).Serve(0x1cae000, 0x1c9a028)
        /root/go/pkg/mod/github.com/mochi-co/mqtt@v1.0.4/server/listeners/tcp.go:89 +0x28
github.com/mochi-co/mqtt/server/listeners.(*Listeners).Serve.func1(0x1cac000, 0x29db60, 0x1cae000, 0x1c9a028)
        /root/go/pkg/mod/github.com/mochi-co/mqtt@v1.0.4/server/listeners/listeners.go:94 +0x70
created by github.com/mochi-co/mqtt/server/listeners.(*Listeners).Serve
        /root/go/pkg/mod/github.com/mochi-co/mqtt@v1.0.4/server/listeners/listeners.go:91 +0x9c
exit status 2

[mochi-co]

Interesting. If I had to guess it's because ARMv7 is a 32bit chip, whereas the atomic counters in the broker are set to use 64bit ints. Is this something you are relying on? I can have a think about how this issue can be fixed, and I welcome suggestions from anyone else!

[ogelami]

Thanks for the quick response! Shouldn't 32-bit be enough for the broker counters?

Could using int where possible and wrapping the sync/atomic with 32/64-bit check for calling say AddUint64 or AddUint32 be a partial solution?

package atomthic

import (
	"sync/atomic"
)

const uintSize = 32 << (^uint(0) >> 32 & 1)

func AddInt(addr *int, delta int) int {
	var c int

	if uintSize == 64 {
		var a int64 = int64(*addr)
		var b = atomic.AddInt64(&a, int64(delta))

		c = int(b)
	} else {
		var a int32 = int32(*addr)
		var b = atomic.AddInt32(&a, int32(delta))

		c = int(b)
	}

	return c
}

func AddUint(addr *uint, delta uint) uint {
	var c uint

	if uintSize == 64 {
		var a uint64 = uint64(*addr)
		var b = atomic.AddUint64(&a, uint64(delta))

		c = uint(b)
	} else {
		var a uint32 = uint32(*addr)
		var b = atomic.AddUint32(&a, uint32(delta))

		c = uint(b)
	}

	return c
}

func LoadInt(addr *int) int {
	var c int

	if uintSize == 64 {
		var a int64 = int64(*addr)
		c = int(atomic.LoadInt64(&a))
	} else {
		var a int32 = int32(*addr)
		c = int(atomic.LoadInt32(&a))
	}

	return c
}

func LoadUint(addr *uint) uint {
	var c uint

	if uintSize == 64 {
		var a uint64 = uint64(*addr)
		c = uint(atomic.LoadUint64(&a))
	} else {
		var a uint32 = uint32(*addr)
		c = uint(atomic.LoadUint32(&a))
	}

	return c
}

func StoreInt(addr *int, val int) {
	if uintSize == 64 {
		var a int64 = int64(*addr)
		var b = int64(val)

		atomic.StoreInt64(&a, b)
	} else {
		var a int32 = int32(*addr)
		var b = int32(val)

		atomic.StoreInt32(&a, b)
	}
}

func StoreUint(addr *uint, val uint) {
	if uintSize == 64 {
		var a uint64 = uint64(*addr)
		var b = uint64(val)

		atomic.StoreUint64(&a, b)
	} else {
		var a uint32 = uint32(*addr)
		var b = uint32(val)

		atomic.StoreUint32(&a, b)
	}
}

[rkennedy]

I'm seeing something similar on what I'm pretty sure is the same hardware. I'm my case, the panic is from an unaligned operation:

/tmp/mqtt $ go run examples/tcp/main.go                                                        
Mochi MQTT Server initializing... TCP                                                          
  Started!                                                                                     
panic: unaligned 64-bit atomic operation                                                       
                                                                                               
goroutine 37 [running]:                                                                        
runtime/internal/atomic.panicUnaligned()                                                       
        /usr/local/go/src/runtime/internal/atomic/unaligned.go:8 +0x24                         
runtime/internal/atomic.Load64(0x8be03c)                                                       
        /usr/local/go/src/runtime/internal/atomic/atomic_arm.s:286 +0x14                       
github.com/mochi-co/mqtt/server/listeners.(*TCP).Serve(0x8be000, 0x8a6028)                     
        /tmp/mqtt/server/listeners/tcp.go:89 +0x28                                             
github.com/mochi-co/mqtt/server/listeners.(*Listeners).Serve.func1(0x8bc000, {0x295934, 0x8be00
0}, 0x8a6028)                                                                                  
        /tmp/mqtt/server/listeners/listeners.go:94 +0x70                                       
created by github.com/mochi-co/mqtt/server/listeners.(*Listeners).Serve                        
        /tmp/mqtt/server/listeners/listeners.go:91 +0x9c                                       
exit status 2

I'm using v1.0.4 with Go 1.17.6.

I'm pretty sure none of the functions in ogelami's reply are atomic anymore, and some of them end up storing into the wrong place. I think you can use unsafe.Sizeof instead of bit-twiddling, but I wonder whether just using uintptr might be a simpler fix. There are functions in atomic for that type already. I tried making that change and it seemed to work. I can submit a PR if you like.

[mochi-co]

I wonder whether just using uintptr might be a simpler fix. There are functions in atomic for that type already. I tried making that change and it seemed to work. I can submit a PR if you like.

@rkennedy I have been thinking along similar lines and would love to see what you've come up with.

I think there might be some trivial improvements to be had by re-aligning the struct declarations across the board, so I am continuing to look into that.

[mochi-co]

After extensive testing, I have released @rkennedy 's fix for this as v1.1.0! Thank you @ogelami and @rkennedy for your hard work and making the project better!