bluetooth icon indicating copy to clipboard operation
bluetooth copied to clipboard

null pointer exception in gap_linux.go#L159

Open rvt opened this issue 3 years ago • 0 comments

HEy,

After testing my application for a relative long-ish time of scanning and receiving data over bluetooth LE received the following SIGVER

2022/08/15 17:33:38 BLE device error :Device-LE : read unix @->/run/dbus/system_bus_socket: EOF
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x653c74]

goroutine 59 [running]:
tinygo.org/x/bluetooth.(*Adapter).Scan(0x4000123080, 0x4000571ed0)
        /home/pi/go_path/pkg/mod/tinygo.org/x/[email protected]/gap_linux.go:159 +0x644
github.com/b3nn0/stratux/v2/gps.(*BleGPSDevice).advertisementListener.func2()
        /home/pi/stratux/gps/bleGPSDevice.go:68 +0x4c
github.com/b3nn0/stratux/v2/gps.(*BleGPSDevice).advertisementListener(0x4000123080, 0x4000200f60)
        /home/pi/stratux/gps/bleGPSDevice.go:90 +0xe8
created by github.com/b3nn0/stratux/v2/gps.(*BleGPSDevice).Run
        /home/pi/stratux/gps/bleGPSDevice.go:278 +0x128

Looking at the code on that line, and possible the comments above it looks like it's possible for a race condition where the channel is closed and we still receive a 0 value of the channel?

The code here : https://github.com/tinygo-org/bluetooth/blob/8dc1e155a0e3a0f7c286a36bce8027bb2b14a650/gap_linux.go#L149 and here https://github.com/tinygo-org/bluetooth/blob/8dc1e155a0e3a0f7c286a36bce8027bb2b14a650/gap_linux.go#L190 Looks like a bit odd to me (to my non trained golang eye)

should this not be look like this all in one select? If it's seperate then it's still possible that a channel get's closed while a closed signal is processed...

for {

	select {
	// Check whether the scan is stopped. This is necessary to avoid a race
	// condition between the signal channel and the cancelScan channel when
	// the callback calls StopScan() (no new callbacks may be called after
	// StopScan is called).
	case <-cancelChan:
		a.adapter.StopDiscovery()
		return nil
	case sig := <-signal:
		// This channel receives anything that we watch for, so we'll have
		// to check for signals that are relevant to us.
		switch sig.Name {
		case "org.freedesktop.DBus.ObjectManager.InterfacesAdded":
			....
		case "org.freedesktop.DBus.Properties.PropertiesChanged":
		.....
		}
}

On https://github.com/tinygo-org/bluetooth/blob/8dc1e155a0e3a0f7c286a36bce8027bb2b14a650/gap_linux.go#L201 Add a call to the cancelChannel with an empty struct

func (a *Adapter) StopScan() error {
	if a.cancelChan == nil {
		return errNotScanning
	}
	a.cancelChan <- struct{}{} // <=== Added
	close(a.cancelChan)
	a.cancelChan = nil
	return nil
}

Another solution is as suggested on the gonuts channel is to use the ok form:

		case sig, ok := <-signal:
			if !ok {
				return nil
			}

rvt avatar Aug 15 '22 18:08 rvt