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

MacOS: cannot use BleakScanner with an event loop in another thread #242

Closed
dhalbert opened this issue Jul 2, 2020 · 9 comments
Closed
Assignees
Labels
Backend: Core Bluetooth Issues and PRs relating to the Core Bluetooth backend bug Something isn't working Documentation Issues or PRs relating to RTD or documentation
Milestone

Comments

@dhalbert
Copy link

dhalbert commented Jul 2, 2020

  • bleak version: 0.7.0
  • Python version: 3.8.2
  • Operating System: MacOS 10.15.5

Thanks for the 0.7.0 release! I no longer have the #206 issue, but due to the way we're using BleakScanner, we're now getting a different error, this time about an internal thread event bleak is using.

I've distilled it down to the test program below. It creates a new event loop in a new thread, and then passes that loop to BleakScanner() to use. This may seem unnecessarily complex, but in the real code we need another thread to allow scanning results to be returned incrementally to the caller.

This program runs fine on Linux under bluez.

The backtrace is below the program:

import asyncio
import bleak
import threading

class Test:
    def __init__(self):
        self.thread = threading.Thread(target=self.run_loop)
        self.thread.daemon = True
        self.thread_ready = threading.Event()
        self.thread.start()
        self.thread_ready.wait()

    def run_loop(self):
        self.loop = asyncio.new_event_loop()
        self.thread_ready.set()
        self.loop.run_forever()

    def await_async(self, coro):
        # Call an async routine from sync code and await its result.
        future = asyncio.run_coroutine_threadsafe(coro, self.loop)
        return future.result(None)

    async def scan(self, timeout):
        await self.scanner.start()
        await asyncio.sleep(timeout)
        await self.scanner.stop()
        return await self.scanner.get_discovered_devices()

    def start_scan(self, timeout):
        self.scanner = bleak.BleakScanner(loop=self.loop)
        for device in self.await_async(self.scan(timeout)):
            print(device)

test = Test()
test.start_scan(5)

This backtrace is generated:

% python3 fail.py
Traceback (most recent call last):
  File "fail.py", line 35, in <module>
    test.start_scan(5)
  File "fail.py", line 31, in start_scan
    for device in self.await_async(self.scan(timeout)):
  File "fail.py", line 21, in await_async
    return future.result(None)
  File "/Users/halbert/.pyenv/versions/3.8.2/lib/python3.8/concurrent/futures/_base.py", line 439, in result
    return self.__get_result()
  File "/Users/halbert/.pyenv/versions/3.8.2/lib/python3.8/concurrent/futures/_base.py", line 388, in __get_result
    raise self._exception
  File "fail.py", line 24, in scan
    await self.scanner.start()
  File "/Users/halbert/.pyenv/versions/3.8.2/lib/python3.8/site-packages/bleak/backends/corebluetooth/scanner.py", line 46, in start
    await self._manager.wait_for_powered_on(0.1)
  File "/Users/halbert/.pyenv/versions/3.8.2/lib/python3.8/site-packages/bleak/backends/corebluetooth/CentralManagerDelegate.py", line 106, in wait_for_powered_on
    await asyncio.wait_for(self.powered_on_event.wait(), timeout)
  File "/Users/halbert/.pyenv/versions/3.8.2/lib/python3.8/asyncio/tasks.py", line 483, in wait_for
    return fut.result()
  File "/Users/halbert/.pyenv/versions/3.8.2/lib/python3.8/asyncio/locks.py", line 309, in wait
    await fut
RuntimeError: Task <Task pending name='Task-2' coro=<Event.wait() running at /Users/halbert/.pyenv/versions/3.8.2/lib/python3.8/asyncio/locks.py:309> cb=[_release_waiter(<Future pendi...1112fb6d0>()]>)() at /Users/halbert/.pyenv/versions/3.8.2/lib/python3.8/asyncio/tasks.py:429]> got Future <Future pending> attached to a different loop
@hbldh hbldh self-assigned this Jul 2, 2020
@hbldh hbldh added this to the Version 0.7.1 milestone Jul 2, 2020
@hbldh hbldh added bug Something isn't working Backend: Core Bluetooth Issues and PRs relating to the Core Bluetooth backend labels Jul 2, 2020
@hbldh
Copy link
Owner

hbldh commented Jul 2, 2020

The #206 resolution was all @dlech, I merely did the merging of it.

This was unfortunate. The got Future <Future pending> attached to a different loop seems very odd and does seem to imply that something is routed incorrectly somewhere.

I haven't got a 10.15 Mac available for the coming 10 hours, but I will look into it after that. I think it is due to that the CentralManagerDelegate created here does not get the loop you created, but calls asyncio.get_event_loop instead and receives the default event loop, upon which it sets the powered on event.

As proposed in #224 the explicit handling of loops should (and will) be removed in Bleak. Have you tried runing the code with self.scanner = bleak.BleakScanner() and avoiding to create a new event loop in run_loop? Might it be solved if Python (and bleak) is allowed to handle the event loops by itself? Or running a asyncio.set_event_loop(self.loop) in run_loop, something I know might not be desired...

@dhalbert
Copy link
Author

dhalbert commented Jul 2, 2020

As proposed in #224 the explicit handling of loops should (and will) be removed in Bleak. Have you tried running the code with self.scanner = bleak.BleakScanner() and avoiding to create a new event loop in run_loop?

Since I need a second thread, and event loops are per-thread, I have to get the one associated with self.thread. But your suggestion inspired me to move the bleak.BleakScanner(loop=self.loop) into scan(), so that the BleakScanner() is created in the new thread, not in the main thread. So that's a workaround, and things are looking much better.

I did try changing self.loop = asyncio.get_event_loop(), because I thought the new loop would be created on demand, but I get an exception that There is no current event loop in thread 'Thread-1'. EDIT: It turns out asyncio.get_event_loop() only creates a new loop if it's in the main thread.

(I now have a whole slew of new problems related to MacOS returning a UUID instead of an actual address, but those are mine to figure out.)

Might it be solved if Python (and bleak) is allowed to handle the event loops by itself?

As long as I can get the event loop when I need it, that's ok. The await_async() routine is my bridge from the sync to the async world, and it needs the loop in order to call asyncio.run_coroutine_threadsafe(). I can comment further on this in #224 (EDIT: not necessary, I think; I was worried #224 is creating loops, but it's just using asyncio.get_event_loop().)

@dlech
Copy link
Collaborator

dlech commented Jul 2, 2020

so that the BleakScanner() is created in the new thread

Yes, everything related to bleak needs to be done in the thread after the new event loop is created.

I now have a whole slew of new problems related to MacOS returning a UUID instead of an actual address

This is a known issue. Apple does not make the Bluetooth address available anywhere for security/privacy reasons.

@dhalbert
Copy link
Author

dhalbert commented Jul 2, 2020

This is a known issue. Apple does not make the Bluetooth address available anywhere for security/privacy reasons.

Yes, thanks, this is something I was not looking forward to tackling. Our CircuitPython libraries make a lot of assumptions about having the address available, and I'll have to think about how to get around this. Perhaps I'll generate a fake address that hashes the UUID down to 48 bits. We have even seen devices we can identify only based on their advertised address. But that's another topic.

@hbldh
Copy link
Owner

hbldh commented Jul 2, 2020

So that's a workaround, and things are looking much better.

Does it mean that you regard this issue as resolved, or would you like to see some changes to how the event loops are handled?

There will be changes to event loop handling in version 0.8.0 which will possibly have breaking changes. You might have to revisit the things you decide to implement now, but my intention is to leave the 0.8.0 release branch open for some days to test before releasing it to PyPI.

Regarding the Bluetooth addresses on Mac, there was issue #140 where @5frank did some preliminary exploration around this. He might have discovered more since then.

@hbldh hbldh modified the milestones: Version 0.7.1, Version 0.7.X Jul 2, 2020
@dhalbert
Copy link
Author

dhalbert commented Jul 3, 2020

Does it mean that you regard this issue as resolved, or would you like to see some changes to how the event loops are handled?

I think it is resolved, except for adding documentation (emphasis below). @dlech can comment if there's anything to fix with regard to the threading.Event that caused the original backtrace error, but my guess is not.

@dlech says:

Yes, everything related to bleak needs to be done in the thread after the new event loop is created.

This is the key point I did not realize, since this restriction isn't necessarily true on the other platforms (or at least my code works by accident on those platforms). It would be good to mention this somewhere in the documentation; that's probably the only action item from this issue.

There will be changes to event loop handling in version 0.8.0 which will possibly have breaking changes. You might have to revisit the things you decide to implement now, but my intention is to leave the 0.8.0 release branch open for some days to test before releasing it to PyPI.

Thanks, I'll certainly test 0.8.0 and modify our code as necessary when it comes out

Regarding the Bluetooth addresses on Mac, there was issue #140 where @5frank did some preliminary exploration around this. He might have discovered more since then.

Thanks for that reference. MacOS/iOS are pretty strict about this security. I'm not sure we want to change our current _bleio API to accommodation a generic ID scheme solely because core Bluetooth works that way, but I think we can work around it somehow.

@dlech
Copy link
Collaborator

dlech commented Jul 3, 2020

@dlech can comment if there's anything to fix with regard to the threading.Event that caused the original backtrace error, but my guess is not.

No, I don't think there is anything to fix here.

This is the key point I did not realize, since this restriction isn't necessarily true on the other platforms (or at least my code works by accident on those platforms). It would be good to mention this somewhere in the documentation; that's probably the only action item from this issue.

Previously, if using the loop= parameters, one could get away with passing around event loops between threads. But some of the recent changes are ignoring the loop parameter since it is going away soon anyway. Not sure what the timeline is for 0.8.0, but is might be good to update the docs now to explain that loop= is going away and how to work around it.

@hbldh hbldh modified the milestones: Version 0.7.X, Version 0.8.0 Jul 9, 2020
@hbldh hbldh added the Documentation Issues or PRs relating to RTD or documentation label Jul 9, 2020
@hbldh
Copy link
Owner

hbldh commented Jul 9, 2020

In the develop branch, there is now a bleak version that does not store and pass any event loops. This change will be present in the 0.8.0 release, so if you want to try it out with regards to this issue, go right ahead!

@hbldh
Copy link
Owner

hbldh commented Sep 25, 2020

Regarding this as done, with the release of version 0.8.0. Good work and thanks for the input on this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend: Core Bluetooth Issues and PRs relating to the Core Bluetooth backend bug Something isn't working Documentation Issues or PRs relating to RTD or documentation
Projects
None yet
Development

No branches or pull requests

3 participants