gh-152635: Raise MemoryError rather than fail assert() when the lock allocation fails in _interpchannels.create() by stestagg · Pull Request #152642 · python/cpython

stestagg · 2026-06-29T23:43:33Z

Previously, an allocation failure when creating the lock for a channel in _interpchannels would trigger an assert. Caused by handle_channel_error being passed an error code of -1 which is only allowed if an exception has been set. (in this case, no exception was set)

channelsmod_create now forwards the error code from channel_create which handle_channel_error already handled.

Because the only way to get a -7 error code (was ERR_CHANNEL_MUTEX_INIT) is via an allocation failure in PyThread_allocate_lock, I made handle_channel_error raise a MemoryError for this case rather than a ChannelError which seems more consistent with general practice.

I also renamed the constant from ERR_CHANNEL_MUTEX_INIT to ERR_CHANNEL_ALLOC_LOCK to make this clearer for future changes.

There's a test to cover this, which does rely on _interpchannels.create code paths not changing too wildly (first allocation has to be the lock), but even if that changes, the test isn't invalid.

Full test suite passes, except test_struct which is known in gh-142414 on MacOS and Windows

Test summary

== Tests result: FAILURE ==

32 tests skipped:
    test.test_asyncio.test_windows_events
    test.test_asyncio.test_windows_utils test.test_gdb.test_backtrace
    test.test_gdb.test_cfunction test.test_gdb.test_cfunction_full
    test.test_gdb.test_jit test.test_gdb.test_misc
    test.test_gdb.test_pretty_print
    test.test_multiprocessing_fork.test_manager
    test.test_multiprocessing_fork.test_misc
    test.test_multiprocessing_fork.test_processes
    test.test_multiprocessing_fork.test_threads
    test.test_os.test_windows test_android test_dbm_gnu test_devpoll
    test_dtrace test_epoll test_free_threading test_idle test_launcher
    test_msvcrt test_startfile test_tcl test_tkinter test_ttk
    test_ttk_textonly test_turtle test_winapi test_winconsoleio
    test_winreg test_wmi

9 tests skipped (resource denied):
    test_curses test_peg_generator test_smtpnet test_socketserver
    test_urllib2net test_urllibnet test_winsound test_xpickle
    test_zipfile64

1 test altered the execution environment (env changed):
    test_httpservers

1 test failed:
    test_struct

462 tests OK.

Issue: _interpchannels.create() debug aborts if channel lock allocation fails #152635

python-cla-bot · 2026-06-29T23:43:38Z

All commit authors signed the Contributor License Agreement.

now raises MemoryError Previously, an allocation failure when creating the lock for a channel in _interpchannels would trigger an assert. Caused by `handle_channel_error` being passed an error code of -1 which is only allowed if an exception has been set. (in this case, no exception was set) `channelsmod_create` now forwards the error code from `channel_create` which `handle_channel_error` already handled. Because the only way to get a -7 error code (was `ERR_CHANNEL_MUTEX_INIT`) is via an allocation failure in `PyThread_allocate_lock`, I made `handle_channel_error` raise a `MemoryError` for this case rather than a `ChannelError`. I also renamed the constant from `ERR_CHANNEL_MUTEX_INIT` to `ERR_CHANNEL_ALLOC_LOCK` to make this clearer for future changes.

…e test method to avoid import issues, improve the syntax for handling the expected MemoryError

has been moved to immediately before the _channels.create() call to ensure we correctly target the allocation failure.

stestagg · 2026-06-30T00:15:46Z

Sorry for the force push, I didn't realise that github would do that with no warning on a simple rebase!

StanFromIreland · 2026-06-30T07:59:30Z

+        cid = None
+        try:
+            with self.assertRaises(MemoryError):
+                _testcapi.set_nomemory(0, 1)


The required start/stop vary by platform, for example on Linux on an unpatched main I get:

$ ./python /tmp/repro.py Traceback (most recent call last): File "/tmp/repro.py", line 6, in <module> cid = _channels.create() ^^^^^^^^^ MemoryError

I'm also against testing with a loop where we try various numbers.

Aah thanks, yes that's annoying, I'd hoped that there would be enough consistency for this to just work.

I noticed that gh-151239 didn't include any tests, maybe for this reason, and that the reproducer in the linked gh-150213 didn't catch this issue, I assume only because they started with n=1.

Having a for-loop over ranges in the tests seems quite comprehensive but maybe overkill for what it is?

I'd appreciate any suggestions here!

In general, we just don't test this due to the associated complexity.

StanFromIreland · 2026-06-30T08:20:30Z

Something went wrong when merging, but we're only bothering Terry (who can unsubscribe in the sidebar ;-) so I don't think we need to recreate.

stestagg · 2026-06-30T08:22:37Z

Something went wrong when merging, but we're only bothering Terry (who can unsubscribe in the sidebar ;-) so I don't think we need to recreate.

Yeah, sorry, I'm working on it, I'm not used to this exact workflow

…ailues seems to be rarely done

sobolevn

Thanks!

To sum up my thoughts:

MemoryError should be raised where the error happens
In this case we should return -1 with the error set
We should remove ERR_CHANNEL_MUTEX_INIT handling, because it will also fail with no memory when trying to create a new exception (probably)
But, I think we should keep the constant, so it's can be reused later, otherwise we would have a gap in numbers. A comment should be enough :)

Just set MemoryError and return directly on alloc failure, rather than go via the handler Leave existing error constant as-is for completeness Co-authored-by: sobolevn <mail@sobolevn.me>

handle_channel_error Tested locally with: def test_lock_allocation_failure(self): import _testcapi for n in range(0, 100): cid = None try: _testcapi.set_nomemory(n, n+1) cid = _channels.create() except MemoryError: pass finally: _testcapi.remove_mem_hooks() if cid is not None: _channels.close(cid, force=True) _channels.destroy(cid)

sobolevn

Thanks!

miss-islington-app · 2026-06-30T10:57:32Z

Thanks @stestagg for the PR, and @sobolevn for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15.
🐍🍒⛏🤖

bedevere-app · 2026-06-30T10:57:45Z

GH-152671 is a backport of this pull request to the 3.15 branch.

bedevere-app · 2026-06-30T10:57:51Z

GH-152672 is a backport of this pull request to the 3.14 branch.

bedevere-app · 2026-06-30T10:57:58Z

GH-152673 is a backport of this pull request to the 3.13 branch.

stestagg · 2026-06-30T10:58:19Z

@sobolevn - Thanks for your help guiding me!

sobolevn · 2026-06-30T10:58:46Z

Happy to help! Thanks for finding this bug :)

stestagg requested a review from ericsnowcurrently as a code owner June 29, 2026 23:43

bedevere-app Bot mentioned this pull request Jun 29, 2026

_interpchannels.create() debug aborts if channel lock allocation fails #152635

Open

bedevere-app Bot added the awaiting review label Jun 29, 2026

stestagg added 3 commits June 30, 2026 01:14

Add @nomemtest decorator to the test, move the _testcapi call into th…

b61a06f

…e test method to avoid import issues, improve the syntax for handling the expected MemoryError

self.assertRaises causes an allocation, so the _testcapi call

191f46a

has been moved to immediately before the _channels.create() call to ensure we correctly target the allocation failure.

stestagg force-pushed the main branch from 8e264d6 to 191f46a Compare June 30, 2026 00:14

StanFromIreland reviewed Jun 30, 2026

View reviewed changes

stestagg requested a review from terryjreedy as a code owner June 30, 2026 08:17

StanFromIreland removed the request for review from terryjreedy June 30, 2026 08:19

Remove the test as it's not reliable enough, and testing allocation f…

aeb50c0

…ailues seems to be rarely done

stestagg force-pushed the main branch from 5508850 to aeb50c0 Compare June 30, 2026 08:28

sobolevn reviewed Jun 30, 2026

View reviewed changes

Comment thread Modules/_interpchannelsmodule.c Outdated

Comment thread Modules/_interpchannelsmodule.c Outdated

Comment thread Modules/_interpchannelsmodule.c

Code review suggestions

b9182d3

Just set MemoryError and return directly on alloc failure, rather than go via the handler Leave existing error constant as-is for completeness Co-authored-by: sobolevn <mail@sobolevn.me>

sobolevn reviewed Jun 30, 2026

View reviewed changes

Comment thread Modules/_interpchannelsmodule.c Outdated

sobolevn approved these changes Jun 30, 2026

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting review labels Jun 30, 2026

sobolevn added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels Jun 30, 2026

sobolevn merged commit b383aa6 into python:main Jun 30, 2026
107 of 110 checks passed

bedevere-app Bot removed the awaiting merge label Jun 30, 2026

bedevere-app Bot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jun 30, 2026

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Jun 30, 2026

bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Jun 30, 2026

Uh oh!

Uh oh!

Conversation

stestagg commented Jun 29, 2026 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

python-cla-bot Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stestagg commented Jun 30, 2026

Uh oh!

StanFromIreland Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stestagg Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

StanFromIreland Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

StanFromIreland commented Jun 30, 2026

Uh oh!

stestagg commented Jun 30, 2026

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miss-islington-app Bot commented Jun 30, 2026

Uh oh!

bedevere-app Bot commented Jun 30, 2026

Uh oh!

bedevere-app Bot commented Jun 30, 2026

Uh oh!

bedevere-app Bot commented Jun 30, 2026

Uh oh!

stestagg commented Jun 30, 2026

Uh oh!

sobolevn commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

stestagg commented Jun 29, 2026 •

edited by bedevere-app Bot

Loading

python-cla-bot Bot commented Jun 29, 2026 •

edited

Loading

StanFromIreland Jun 30, 2026 •

edited

Loading