dotintent / FlutterBleLib

Bluetooth Low Energy library for Flutter with support for simulating peripherals
Apache License 2.0
531 stars 195 forks source link

Leaking BleAdapter #563

Open JamesMcIntosh opened 3 years ago

JamesMcIntosh commented 3 years ago

If you create multiple clients when FlutterBleLibPlugin.createClient() overwrites the existing BleAdapter.

This means you can never destroy the previous clients, I'm pretty sure that this could cause a memory leaks as there are listeners attached etc.

If you guard against creating a second one then bleAdapter.createClient() will need to be skipped also as it will cause the same issue with leaks as BleModule is written assuming the same pattern which will overwrite the client, see BleModule.java#L110

duck-dev-go commented 3 years ago

@JamesMcIntosh did you test if this happens with the memory profiler?

JamesMcIntosh commented 3 years ago

Hi @michael-ottink, I don't think that I checked it with a profiler, I came to this suggestion from a visual inspection of the code and the reported behaviour around hot restarts.

JamesMcIntosh commented 3 years ago

I profiled it and it is leaking as expected.

JamesMcIntosh commented 3 years ago

To get around the hot restart issue the createClient method can either destroy and recreate or reuse the existing one.

After testing out the destroyClient method and it looks like the destroy it still leaks.

The leak originates in MultiPlatformBleAdapter is because of RxJava1's subscriptions to Observers holds strong references to the anonymous inner classes inside BleModule i.e onError and onNext.

More information: https://medium.com/@scanarch/how-to-leak-memory-with-subscriptions-in-rxjava-ae0ef01ad361