Skip to content

loaders/py_loader: fix asyncio shutdown deadlock and SIGPIPE crash (#803) - #912

Open
anmoltrivedi2312-ux wants to merge 6 commits into
metacall:developfrom
anmoltrivedi2312-ux:fix-py-asyncio-deadlock-803
Open

anmoltrivedi2312-ux wants to merge 6 commits into
metacall:developfrom
anmoltrivedi2312-ux:fix-py-asyncio-deadlock-803

Conversation

@anmoltrivedi2312-ux

Copy link
Copy Markdown
Contributor

Description
Problem
Shutdown Deadlock: The Python loader previously used threading.Thread to execute the background asyncio event loop. During runtime shutdown, the main thread attempted to join the Python thread while holding the GIL. The worker thread required the GIL to process the loop.stop() cleanup and terminate, causing a circular deadlock. Skipping the join prevented the hang but caused segmentation faults due to interpreter finalization running while the thread was still active.

Intermittent SIGPIPE: During event loop socketpair closure on POSIX systems, flushes on closed pipe descriptors generated unhandled SIGPIPE signals, terminating the host process.

Solution
Native OS Threading Abstraction (source/threading): Added a cross-platform C-level thread API (threading_thread.h) implemented via POSIX pthreads (threading_thread_unix.c) and Windows CRT _beginthreadex (threading_thread_win32.c).

C-Managed Event Loop: Migrated the background asyncio loop execution to run inside a native OS thread managed from C land.

Deadlock-Free Teardown: In py_loader_impl_finalize_asyncio_module, the GIL is released via Py_BEGIN_ALLOW_THREADS before invoking threading_thread_join(), ensuring the background thread can acquire Python thread state, exit cleanly, and join without contention.

Signal Handling: Set signal(SIGPIPE, SIG_IGN) on POSIX systems during initialization to prevent abrupt process termination during socket teardown.

Legacy Cleanup: Removed obsolete Windows thread inspection routines.

Verification & Testing
Regression: All 12 metacall-python-* test targets pass without failure (100% pass rate).

Stress Testing: Executed 25 consecutive iterations of metacall-python-async-test with zero hangs or deadlocks.

Helgrind (Valgrind): Validated lock order and destruction sequence; confirmed clean thread destruction prior to Py_FinalizeEx.

ThreadSanitizer (TSan): Executed 10 stress passes of metacall-python-async-test and the full Python test suite under -fsanitize=thread; 0 data races detected.

Closes #803

- Implement cross-platform OS thread abstraction in source/threading (POSIX pthreads, Win32 _beginthreadex).
- Migrate asyncio background loop execution to a native OS thread.
- Release GIL with Py_BEGIN_ALLOW_THREADS prior to joining native thread on loader destruction.
- Ignore SIGPIPE on POSIX systems to prevent process abort during asyncio socketpair teardown flushes.
- Clean up legacy Windows thread inspection workarounds.

Closes metacall#803
@anmoltrivedi2312-ux
anmoltrivedi2312-ux marked this pull request as draft September 27, 2026 05:55
Comment thread source/threading/CMakeLists.txt Outdated

INTERFACE
)
#

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.

can you keep the line endings as the original one?

@viferga

viferga commented Sep 27, 2026

Copy link
Copy Markdown
Member

Can you explain this better?

Intermittent SIGPIPE: During event loop socketpair closure on POSIX systems, flushes on closed pipe descriptors generated unhandled SIGPIPE signals, terminating the host process.

This haven't been observed before.

@viferga

viferga commented Sep 27, 2026

Copy link
Copy Markdown
Member

The code looks great overall.

@anmoltrivedi2312-ux

Copy link
Copy Markdown
Contributor Author

@viferga i will let u know its giving errors in ci right now. work in progress.

@viferga

viferga commented Sep 27, 2026

Copy link
Copy Markdown
Member

🫡

@anmoltrivedi2312-ux
anmoltrivedi2312-ux marked this pull request as ready for review September 30, 2026 15:26

if (host == 1)
{
threading_thread_detach(&py_impl->asyncio_thread);

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.

I'm not sure about this, I have to think about it

@anmoltrivedi2312-ux anmoltrivedi2312-ux Oct 1, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Take your time
The detach was just to avoid deadlocks on shutdown if asyncio tasks were still pending

This branch has not been deployed

No deployments
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.

Python loader segfaults on FreeBSD

2 participants