Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ScanOperationApi21.java - Fixing the memory leak from ScanOperationApi21 emitter #708

Merged
merged 2 commits into from
Dec 11, 2020

Conversation

seotrader
Copy link
Contributor

Fixing the memory leak from ScanOperationApi21 emitter
Fixing the issue: #607

Fixing the memory leak from ScanOperationApi21 emitter
@CLAassistant
Copy link

CLAassistant commented Jul 31, 2020

CLA assistant check
All committers have signed the CLA.

@dariuszseweryn
Copy link
Owner

Hello!
I am trying to get back to this issue with the author of Leak Canary as I have some doubts.
There is one flaw in your PR — it is prone to race condition when the callback is being nullified on a different thread.

@seotrader
Copy link
Contributor Author

Hi @dariuszseweryn , any updates from Leak Canary?

Regarding to the race condition on the callback. Do you have a preferred way for me to fix it?

Thanks

@dariuszseweryn
Copy link
Owner

No updates yet, I am fully occupied :/
As for the preferred way — a simple fix would be to have a local reference in each function before checking if the recipient is not null.

@b055man
Copy link

b055man commented Sep 16, 2020

@dariuszseweryn I'm observing a problem with the library after prolonged used, namely after several hours spent on reconnect attempts (no scanning, but thousands of connect attempts) - it looks like afterwards flutter_ble_lib is not operational anymore and any attempt to initate scanning fails instantly as the stream is closed (onDone is called instantly).
Looking at Android Studio noticed this:
image
This is just a heads up - I will try to provide more data and observations. For now, just a question if you've seen something smilar before?

- Keeping ref to scanEmitter inside the functions to avoid race condition when the callback is being nullified on a different thread.
@seotrader
Copy link
Contributor Author

@dariuszseweryn made another PR with the fix you wanted

I don't see any leak with LeakCanary after calling scanBleDevices() and closing the activity

image

@dariuszseweryn
Copy link
Owner

@b055man Have you reported this issue on Flutter lib repo? I have not seen anything like that yet.
@seotrader Merging. 👍 Thank you

@dariuszseweryn dariuszseweryn merged commit 75b854f into dariuszseweryn:master Dec 11, 2020
@b055man
Copy link

b055man commented Dec 11, 2020

@dariuszseweryn Sorry for not updating this. The root cause for this problem has been identified already - I've an issue here: dotintent/FlutterBleLib#528 The original cause has been resolved in the RxAndroidBle RxJava2-based version of the lib but FlutterBleLIb/MultiplatformBleAdapter was (still is, to be precise) based on the earlier version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants