Skip to content

Try to debug non-closed iopub socket - #1345

Merged
Carreau merged 4 commits into
ipython:mainfrom
Carreau:iopubclose
Feb 19, 2025
Merged

Try to debug non-closed iopub socket#1345
Carreau merged 4 commits into
ipython:mainfrom
Carreau:iopubclose

Conversation

@Carreau

@CarreauCarreau commented Feb 18, 2025

Copy link
Copy Markdown
Member

We sometime get

 FAILED tests/test_connect.py::test_port_bind_failure_gives_up_retries - pytest.PytestUnraisableExceptionWarning: Exception ignored in: <function Socket.__del__ at 0x7efe22e4d760>
Traceback (most recent call last):
File "/opt/hostedtoolcache/Python/3.12.9/x64/lib/python3.12/site-packages/zmq/sugar/socket.py", line 185, in __del__
warn(
ResourceWarning: Unclosed socket <zmq.asyncio.Socket(zmq.PUB) at 0x7efe1eb32820><F48>

This adds a tracemalloc fixture for the crashing test module in order to try to narrow that down. I'm thorn between restart the tests here until we have the failure; or merge and see once we get the error in another PR and then remove the fixture once we've fund the culprit.


The only zmq.PUB I can find is iopub, and we don't seem to be closing it

In addition : should app be a contextmanager ?
And should the atexit close registring be replaced with a weakref finalizer ?

We sometime get
```
FAILED tests/test_connect.py::test_port_bind_failure_gives_up_retries - pytest.PytestUnraisableExceptionWarning: Exception ignored in: <function Socket.__del__ at 0x7efe22e4d760>
Traceback (most recent call last):
File "/opt/hostedtoolcache/Python/3.12.9/x64/lib/python3.12/site-packages/zmq/sugar/socket.py", line 185, in __del__
warn(
ResourceWarning: Unclosed socket <zmq.asyncio.Socket(zmq.PUB) at 0x7efe1eb32820><F48>
```
The only zmq.PUB I can find is iopub, and we don't seem to be closing it
In addition : should app be a contextmanager ?
And should the atexit close registring be replaced with a weakref
finalizer ?
t
@Carreau
Carreau requested a review from minrkFebruary 18, 2025 14:14
@CarreauCarreau added the bug label Feb 18, 2025
@Carreau

Copy link
Copy Markdown
MemberAuthor

Ah ! No, in the real application the iopub_socket is closed when you close the thread, but in conftest it is not !

@Carreau

Copy link
Copy Markdown
MemberAuthor

well, in conf.py it is also closed...

@Carreau

Copy link
Copy Markdown
MemberAuthor

@minrk do you believe that using tracemalloc in the del in pyzmq to print where the socket was created could help and would you accept a PR ?

@minrk

Copy link
Copy Markdown
Member

@Carreau 100%! I thought you had to -X tracemalloc / PYTHONTRACEMALLOC=1 to get this output from ResourceWarnings. Can we run the tests with that?

@Carreau

Copy link
Copy Markdown
MemberAuthor

@Carreau 100%! I thought you had to -X tracemalloc / PYTHONTRACEMALLOC=1 to get this output from ResourceWarnings. Can we run the tests with that?

I think you need both ? My understanding is that -X tracemalloc / PYTHONTRACEMALLOC=1 just enable tracemalloc, but that you still need to call tracemalloc.get_object_traceback(obj) in the code that emit the warnings, and tracemalloc.get_object_traceback(obj) ?

Maybe I'm wrong.

@Carreau

Carreau commented Feb 19, 2025

Copy link
Copy Markdown
MemberAuthor

Seem that you are right it shoudl show up automatically. I will try differently.

@CarreauCarreau changed the title Always close iopub socketTry to debug non-closed iopub socketFeb 19, 2025
@Carreau

Copy link
Copy Markdown
MemberAuthor

Thanks for the review. I'm going to merge and hope it triggers a more helpful error message soon.

@CarreauCarreau added this to the 7.0 milestone Feb 19, 2025
@Carreau
Carreau merged commit 486050f into ipython:mainFeb 19, 2025
ianthomas23 pushed a commit to ianthomas23/ipykernel that referenced this pull request Jul 14, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Carreau@minrk