GeezerVison

The World Wide Web, restored to 800 pixels of glory.

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

Conversation

stestagg commented Jun 29, 2026 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

stestagg requested a review from ericsnowcurrently as a code owner

June 29, 2026 23:43

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

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

All commit authors signed the Contributor License Agreement.

stestagg reacted with rocket emoji

All reactions

🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot mentioned this pull request

Jun 29, 2026

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

Closed

bedevere-app Bot added the awaiting review label

Jun 29, 2026

stestagg added 3 commits

June 30, 2026 01:14


          Fix: pythongh-152635 _interpchannels channel lock alloc failure

c25ac11

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.


          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

stestagg commented Jun 30, 2026

Copy link

Copy Markdown

Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

StanFromIreland reviewed

Jun 30, 2026

View reviewed changes

Comment thread

Lib/test/test_interpreters/test_channels.py Outdated

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

StanFromIreland Jun 30, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

stestagg Jun 30, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

StanFromIreland Jun 30, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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

StanFromIreland commented Jun 30, 2026

Copy link

Copy Markdown

Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

stestagg commented Jun 30, 2026

Copy link

Copy Markdown

Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          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

sobolevn left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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 :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Modules/_interpchannelsmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/_interpchannelsmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/_interpchannelsmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.


          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 <[email protected]>

sobolevn reviewed

Jun 30, 2026

View reviewed changes

Comment thread

Modules/_interpchannelsmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Remove the now-redundant (and incorrect) branch from

25b178a

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 approved these changes

Jun 30, 2026

View reviewed changes

sobolevn left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

stestagg reacted with hooray emoji

All reactions

🎉 1 reaction

bedevere-app Bot added awaiting merge and removed awaiting review labels

Jun 30, 2026

sobolevn added needs backport to 3.13

only security fixes

needs backport to 3.14

bugs and security fixes

needs backport to 3.15 bugs and security fixes labels

Jun 30, 2026

sobolevn merged commit b383aa6 into python:main

Jun 30, 2026

107 of 110 checks passed

Uh oh!

There was an error while loading. Please reload this page.

miss-islington-app Bot commented Jun 30, 2026

Copy link

Copy Markdown

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.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the awaiting merge label

Jun 30, 2026

bedevere-app Bot commented Jun 30, 2026

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label

Jun 30, 2026

bedevere-app Bot commented Jun 30, 2026

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label

Jun 30, 2026

bedevere-app Bot commented Jun 30, 2026

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the needs backport to 3.13 only security fixes label

Jun 30, 2026

stestagg commented Jun 30, 2026

Copy link

Copy Markdown

Contributor Author

@sobolevn - Thanks for your help guiding me!

sobolevn reacted with heart emoji

All reactions

❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sobolevn commented Jun 30, 2026

Copy link

Copy Markdown

Member

Happy to help! Thanks for finding this bug :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sobolevn added a commit that referenced this pull request

Jun 30, 2026


          [3.13] gh-152635: Raise MemoryError when the lock allocation fails in…

2c9972c

… `_interpchannels.create()` (GH-152642) (#152673)

gh-152635: Raise MemoryError when the lock allocation fails in `_interpchannels.create()` (GH-152642)

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.
(cherry picked from commit b383aa6)

Co-authored-by: Steve Stagg <[email protected]>
Co-authored-by: sobolevn <[email protected]>

sobolevn added a commit that referenced this pull request

Jun 30, 2026


          [3.15] gh-152635: Raise MemoryError when the lock allocation fails in…

0d76fdb

… `_interpchannels.create()` (GH-152642) (#152671)

gh-152635: Raise MemoryError when the lock allocation fails in `_interpchannels.create()` (GH-152642)

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.
(cherry picked from commit b383aa6)

Co-authored-by: Steve Stagg <[email protected]>
Co-authored-by: sobolevn <[email protected]>

sobolevn added a commit that referenced this pull request

Jun 30, 2026


          [3.14] gh-152635: Raise MemoryError when the lock allocation fails in…

589833e

… `_interpchannels.create()` (GH-152642) (#152672)

gh-152635: Raise MemoryError when the lock allocation fails in `_interpchannels.create()` (GH-152642)

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.
(cherry picked from commit b383aa6)

Co-authored-by: Steve Stagg <[email protected]>
Co-authored-by: sobolevn <[email protected]>

maurycy added a commit to maurycy/cpython that referenced this pull request

Jul 1, 2026


          Merge branch 'main' into maurycy/bye-abba

da7d768

* main: (266 commits)
  pythongh-151626: Fix tests that fail when PYTHONPYCACHEPREFIX is set (pythonGH-151952)
  pythongh-152728: IDLE - move 3 toplevel fix_xyz functions to idlelb.util (python#152729)
  pythongh-152711: Add pythoninfo-build command to Platforms/Android (python#152713)
  pythongh-152715: Add pythoninfo-build command to Platforms/Apple (python#152716)
  pythongh-152433: Windows: enable mmapmodule for UWP (python#152473)
  pythongh-152433: Windows: use GetFileSizeEx instead of GetFileSize for memory mapped files (python#152383)
  pythonGH-81881: Raise `SpecialFileError` for sockets and devices in `shutil.copyfile` (python#142693)
  pythongh-152502: Detect the curses mouse interface and is_* methods portably (pythonGH-152705)
  pythongh-145857: Replace `DELETE_GLOBAL` with `PUSH_NULL; STORE_GLOBAL` (pythonGH-146314)
  pythongh-145854: Replace `DELETE_NAME` with `PUSH_NULL; STORE_NAME` (pythonGH-146006)
  pythongh-152680: Detect container/VM in test.pythoninfo (python#152668)
  pythongh-152682: Fix NULL dereference on OOM in `symtable_visit_type_param_bound_or_default` (python#152684)
  pythongh-151881: Skip tk_inactive negativity check on Windows (pythonGH-152683)
  pythongh-152546: Refactor `mappingproxy.__new__` to use `PyDictProxy_New` (python#152547)
  pythongh-151126: Fix a possible crash during the startup with no memory under `Py_STACKREF_DEBUG` (python#152478)
  pythongh-152635: Raise MemoryError when the lock allocation fails in `_interpchannels.create()` (python#152642)
  pythongh-151029: Fix `test_remote_exec_deleted_static_executable` on static installed builds (pythonGH-152653)
  pythongh-121249: Deprecate using F/D type codes in the struct module (python#152309)
  pythongh-152192: Fix JUMP_BACKWARD passing a truncated oparg to the jit tracer (pythonGH-152382)
  Don't require the `_test{internal}capi` modules in `test_monitoring.py` (python#152311)
  ...

IvyXu420 reviewed

Jul 9, 2026

View reviewed changes

Comment thread

Modules/_interpchannelsmodule.c

		int64_t cid = channel_create(&_globals.channels, defaults);
		if (cid < 0) {
		(void)handle_channel_error(-1, self, cid);
		(void)handle_channel_error(cid, self, cid);

IvyXu420 Jul 9, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It causes a warning when compiling CPython using MSVC on Windows x64.
e.g., in GitHub Actions:

[Build CPython: Modules/_interpchannelsmodule.c#L2938]
'function': conversion from 'int64_t' to 'int', possible loss of data [D:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

sobolevn Jul 9, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I will fix it, thanks for the report!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

This was referenced Jul 9, 2026

Fix compile warning "Modules/_interpchannelsmodule.c: conversion from 'int64_t' to 'int', possible loss of data" #153444

Closed

gh-153444: Fix compile warning in Modules/_interpchannelsmodule.c #153445

Merged

Reviewers

sobolevn

sobolevn approved these changes

StanFromIreland

StanFromIreland left review comments

ericsnowcurrently

Awaiting requested review from ericsnowcurrently

ericsnowcurrently is a code owner

+1 more reviewer

IvyXu420

IvyXu420 left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

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!

IvyXu420 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

sobolevn Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stestagg commented Jun 29, 2026 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

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

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

StanFromIreland Jun 30, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.