tinygo-org / bluetooth

Cross-platform Bluetooth API for Go and TinyGo. Supports Linux, macOS, Windows, and bare metal using Nordic SoftDevice or HCI
https://tinygo.org
Other
728 stars 136 forks source link

Memory leak on Linux with repeated device connections #252

Open jasday opened 6 months ago

jasday commented 6 months ago

Expanding on the example heartbeat-monitor, I'm getting what appears to be a memory leak when re-connecting to a device after the first connection.

An example use case:

Example code, edited slightly from the source:

func (o *Bluetooth) Start() error {
        println("enabling")

    // Enable BLE interface.
    must("enable BLE stack", adapter.Enable())

    // Start scanning.
    println("scanning...")
        // Added a loop so we can start the scan again once we've finished with the current device
        for {
        ch := make(chan bluetooth.ScanResult, 1)

        // Start scanning.
        println("scanning...")
        err = adapter.Scan(func(adapter *bluetooth.Adapter, result bluetooth.ScanResult) {
            println("found device:", result.Address.String(), result.RSSI, result.LocalName())
                        // Using a health thermometer service as a test here, can be swapped out with anything else
            if result.AdvertisementPayload.HasServiceUUID(bluetooth.ServiceUUIDHealthThermometer) {
                adapter.StopScan()
                ch <- result
            }
        })
                var device *bluetooth.Device

        select {
        case result := <-ch:
            device, err := adapter.Connect(result.Address, bluetooth.ConnectionParams{})
            if err != nil {
                println(err.Error())
                return err
            }

            println("connected to ", result.Address.String())
            // Handle some logic here on the device
            println("Disconnected")
            device.Disconnect()
        }
    }

}

Go version: 1.22.1 tinygo/x/bluetooth: 0.8.0

Compiling onto Linux Arm64, with a IW416 SOC

Am I doing anything wrong based on the example code? The only real difference here is the added loop, which (hopefully) allows for scanning and connecting to different devices. (I need to be able to connect to multiple devices with various different services at unknown times, so this is just attempting to get the basics working, so I need the ability to restart the scan once the operation on the current device is finished)

Any ideas?

asteria-jacky commented 6 months ago

I think the example code is good, instead there could be a problem in gap_linux.go. I see the number of goroutines keep incrasing together with memory usage, even after trying to connect once.

I can't see anywhere (*device.Device1).UnwatchProperties get called while there is (*device.Device1)WatchProperties. This can make underlying dbus signal channel not get unregistered, then result in leaking a goroutine for every single deffered undelivered signal from dbus. https://github.com/tinygo-org/bluetooth/blob/d0c7887b81e136df90b732d1162cdb335633aafa/gap_linux.go#L326-L331

I forked and added a defer d.device.UnwatchProperties(d.propchanged) and it seems okay so far.

They are trying to remove the dependency on muka/bluetooth, so I doubt this will be fixed.

jasday commented 6 months ago

This seems to have been fixed in the latest v0.9.0 - When simply connecting to a device and disconnecting the memory no longer increases and will remain fairly stable. There is however increasing memory usage when connecting to a device and enabling notifications as mentioned in https://github.com/tinygo-org/bluetooth/issues/260.