Skip to content

Do not use asyncio in weakref.finalize#562

Open
tfarago wants to merge 2 commits into
masterfrom
fix-uca-finalize
Open

Do not use asyncio in weakref.finalize#562
tfarago wants to merge 2 commits into
masterfrom
fix-uca-finalize

Conversation

@tfarago

@tfarago tfarago commented Aug 27, 2025

Copy link
Copy Markdown
Contributor

because the ThreadPoolExecutor of asyncio streams complains about submitting things after interpreter shutdown. Instead of trying to be super-clever, I just use the standard sockets, which work and there is no magic. @MarcusZuber this is one of the reasons why async destructors would bite us in our asses in the future.

Traceback (most recent call last):
  File "/usr/lib/python3.12/weakref.py", line 666, in _exitfunc
    f()
  File "/usr/lib/python3.12/weakref.py", line 590, in __call__
    return info.func(*info.args, **(info.kwargs or {}))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tomas/.local/lib/python3.12/site-packages/concert/devices/cameras/uca.py", line 306, in _ucad_unregister_all
    asyncio.run(_ucad_communicate(struct.pack("I", 14), host, port))
  File "/usr/lib/python3.12/asyncio/runners.py", line 194, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 687, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/home/tomas/.local/lib/python3.12/site-packages/concert/devices/cameras/uca.py", line 295, in _ucad_communicate
    reader, writer = await asyncio.open_connection(host, port)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/streams.py", line 48, in open_connection
    transport, _ = await loop.create_connection(
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 1080, in create_connection
    infos = await self._ensure_resolved(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 1456, in _ensure_resolved
    return await loop.getaddrinfo(host, port, family=family, type=type,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 901, in getaddrinfo
    return await self.run_in_executor(
                 ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 863, in run_in_executor
    executor.submit(func, *args), loop=self)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/concurrent/futures/thread.py", line 172, in submit
    raise RuntimeError('cannot schedule new futures after '
RuntimeError: cannot schedule new futures after interpreter shutdown

Alternative solution (which would enable custom concert shutdown hook)

Rough code:

$ concert start adestruct
adestruct > from concert.devices.cameras.uca import RemoteNetCamera, _ucad_unregister_all
adestruct > def mine(info):
       ...:     if info.raw_cell == "exit":
       ...:         _ucad_unregister_all("localhost", 8989)
       ...: 
adestruct > ip = get_ipython()
adestruct > ip.events.register("pre_run_cell", mine)
adestruct > a = 1
# Nothing happens
adestruct > exit
# on ucad: ** (ucad:205608): DEBUG: 12:28:02.831: All endpoints removed

Actual simple concert implementation is in this commit.

because the ThreadPoolExecutor of asyncio streams complains:

Traceback (most recent call last):
  File "/usr/lib/python3.12/weakref.py", line 666, in _exitfunc
    f()
  File "/usr/lib/python3.12/weakref.py", line 590, in __call__
    return info.func(*info.args, **(info.kwargs or {}))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tomas/.local/lib/python3.12/site-packages/concert/devices/cameras/uca.py", line 306, in _ucad_unregister_all
    asyncio.run(_ucad_communicate(struct.pack("I", 14), host, port))
  File "/usr/lib/python3.12/asyncio/runners.py", line 194, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 687, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/home/tomas/.local/lib/python3.12/site-packages/concert/devices/cameras/uca.py", line 295, in _ucad_communicate
    reader, writer = await asyncio.open_connection(host, port)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/streams.py", line 48, in open_connection
    transport, _ = await loop.create_connection(
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 1080, in create_connection
    infos = await self._ensure_resolved(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 1456, in _ensure_resolved
    return await loop.getaddrinfo(host, port, family=family, type=type,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 901, in getaddrinfo
    return await self.run_in_executor(
                 ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 863, in run_in_executor
    executor.submit(func, *args), loop=self)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/concurrent/futures/thread.py", line 172, in submit
    raise RuntimeError('cannot schedule new futures after '
RuntimeError: cannot schedule new futures after interpreter shutdown
@tfarago tfarago self-assigned this Aug 27, 2025
@tfarago

tfarago commented Aug 27, 2025

Copy link
Copy Markdown
Contributor Author

If you guys agree I would roll this out ASAP, it causes a page-long error on concert exit.

@codecov

codecov Bot commented Aug 27, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.52%. Comparing base (64508fd) to head (2c951a3).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #562   +/-   ##
=======================================
  Coverage   83.52%   83.52%           
=======================================
  Files         133      133           
  Lines       10718    10718           
=======================================
  Hits         8952     8952           
  Misses       1766     1766           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tfarago

tfarago commented Aug 28, 2025

Copy link
Copy Markdown
Contributor Author

@MarcusZuber the "alternative" solution would enable async destructors, not per se, but by registering some cleanup actions in concert.

@MarcusZuber MarcusZuber left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes it here and we could merge it.

Having the possibility to use "proper" destructors would also be nice to have.

@tfarago

tfarago commented Sep 11, 2025

Copy link
Copy Markdown
Contributor Author

I am a little bit afraid when it comes to the ipython magic but it avoids duplicating async code with a sync version. So I will implement that and in the worst case we change it later.

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.

2 participants