Suppress warning for specifically handled behavior

[cottsay]

In Python 3.10, there is a deprecation warning raised when calling get_event_loop when there is no running loop. In the future, it will raise a RuntimeError, which is already handled by this code in exactly the way we want it to.

Once we drop support for python < 3.8, we can change this function to get_running_loop and drop the warning filter.

[clalancette]

Oh, nice work on this. This is one of a few places in ROS 2 that have this issue.

That said, I wonder if we should use this PR to introduce the call to get_running_loop. That way when we remove support for older Python, we just have to remove code, not add anything new. I understand that get_running_loop is a new API in Python 3.7, so we'll probably have to catch an AttributeError as well.

What do you think?

[cottsay]

What do you think?

I hadn't thought about catching the AttributeError in Python < 3.7 - that's not a bad idea. I'll update the PR and we'll see how it looks.

In any case, we should get the GitHub Actions online to fully test this change before merging it.

[cottsay]

That said, I wonder if we should use this PR to introduce the call to get_running_loop.

After thinking about it, I'm not sure we want to do this. In older Python versions, get_running_loop will never succeed, so each time this code is processed, it would create a new event loop and would never find the running loop - at least not without some additional nested handling.

I think it might muddy things even worse to introduce get_running_loop now, but feel free to update this PR with a suggestion if you see a way to do it cleanly.

[clalancette]

So I think it makes sense to step back a bit and describe what the get_loop_impl function is fundamentally trying to do.

From my reading here, what it seems to be doing is a one-time setup of loop to make sure it is setup how we want, and is actually set. That's not totally clear from the current logic, so here is a rewrite of the function that I think makes that a bit clearer, while also dealing with the possibility that the python version doesn't have get_running_loop:

def get_loop_impl(asyncio):
    global _thread_local
    if getattr(_thread_local, 'loop_has_been_setup', False):
        return asyncio.get_event_loop()

    # Setup this thread's loop and return it
    try:
        loop = asyncio.get_running_loop()
        have_running_loop = True
    except RuntimeError:
        # Thrown when there is no running loop
        have_running_loop = False
    except AttributeError:
        # Thrown when on Python < 3.7
        loop = asyncio.get_event_loop()
        have_running_loop = True

    if have_running_loop:
        if os.name == 'nt':
            if not isinstance(loop, asyncio.ProactorEventLoop):
                # Before replacing the existing loop, explicitly
                # close it to prevent an implicit close during
                # garbage collection, which may or may not be a
                # problem depending on the loop implementation.
                loop.close()
                loop = asyncio.ProactorEventLoop()
    else:
        if os.name == 'nt':
            loop = asyncio.ProactorEventLoop()
        else:
            loop = asyncio.new_event_loop()

    asyncio.set_event_loop(loop)

    _thread_local.loop_has_been_setup = True

    return loop

In the future, when we remove support for Python < 3.7, we can just remove that AttributeError block, and the rest stays the same.

What do you think?

[clalancette]

CI:

• Linux
• Linux-aarch64
• Windows