◐ Shell
reader mode source ↗
Skip to content

gh-104469: Disallow using Py_LIMITED_API with Py_BUILD_CORE#109690

Merged
vstinner merged 1 commit into
python:mainfrom
vstinner:limited_core
Sep 21, 2023
Merged

gh-104469: Disallow using Py_LIMITED_API with Py_BUILD_CORE#109690
vstinner merged 1 commit into
python:mainfrom
vstinner:limited_core

Conversation

@vstinner

@vstinner vstinner commented Sep 21, 2023 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

@bedevere-app bedevere-app Bot mentioned this pull request Sep 21, 2023
Closed
@vstinner

vstinner commented Sep 21, 2023

Copy link
Copy Markdown
Member Author

When I reviewed #107857 I noticed that Py_LIMITED_API was used with Py_BUILD_CORE which makes no sense to me. Make a choice: either the limited C API, or the internal C API :-)

@vstinner

vstinner commented Sep 21, 2023

Copy link
Copy Markdown
Member Author

@corona10 wrote:

Okay, this is an issue between AC and limited C APIs. We should revert it first.
cc @erlend-aasland @vstinner @nahyeon-an

In the meanwhile, AC got support for the limited C API ;-)

@vstinner
pythongh-104469: Disallow using Py_LIMITED_API with Py_BUILD_CORE …
3dcbb46 
Fix make check-c-globals: complete USE_LIMITED_C_API list of the
c-analyzer.
@vstinner vstinner merged commit 3f5c564 into python:main Sep 21, 2023
@vstinner vstinner deleted the limited_core branch September 21, 2023 23:21
@vstinner

vstinner commented Sep 21, 2023

Copy link
Copy Markdown
Member Author

Merged. Thanks for the review @colesbury.

@bedevere-bot

bedevere-bot commented Sep 22, 2023

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot s390x RHEL7 LTO + PGO 3.x has failed when building commit 3f5c564.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/244/builds/5490) and take a look at the build logs.
  4. Check if the failure is related to this commit (3f5c564) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/244/builds/5490

Failed tests:

  • test.test_concurrent_futures.test_shutdown

Failed subtests:

  • test_interpreter_shutdown - test.test_concurrent_futures.test_shutdown.ProcessPoolSpawnProcessPoolShutdownTest.test_interpreter_shutdown

Summary of the results of the build (if available):

==

Click to see traceback logs 
Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/test/test_concurrent_futures/test_shutdown.py", line 49, in test_interpreter_shutdown
    self.assertFalse(err)
AssertionError: b'Exception in thread Thread-1:\nTraceback (most recent call last):\n  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/threading.py", line 1059, in _bootstrap_inner\n    self.run()\n  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/concurrent/futures/process.py", line 339, in run\n    self.add_call_item_to_queue()\n  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/concurrent/futures/process.py", line 394, in add_call_item_to_queue\n    self.call_queue.put(_CallItem(work_id,\n  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/multiprocessing/queues.py", line 94, in put\n    self._start_thread()\n  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/multiprocessing/queues.py", line 177, in _start_thread\n    self._thread.start()\n  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/threading.py", line 978, in start\n    _start_new_thread(self._bootstrap, ())\nRuntimeError: can\'t create new thread at interpreter shutdown\nTraceback (most recent call last):\n  File "<string>", line 1, in <module>\n  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/multiprocessing/spawn.py", line 122, in spawn_main\n    exitcode = _main(fd, parent_sentinel)\n               ^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/multiprocessing/spawn.py", line 132, in _main\n    self = reduction.pickle.load(from_parent)\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/multiprocessing/synchronize.py", line 115, in __setstate__\n    self._semlock = _multiprocessing.SemLock._rebuild(*state)\n                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\nFileNotFoundError: [Errno 2] No such file or directory\n' is not false

@erlend-aasland

erlend-aasland commented Sep 22, 2023

Copy link
Copy Markdown
Contributor

Thanks! LGTM

@vstinner

vstinner commented Sep 22, 2023

Copy link
Copy Markdown
Member Author

Oh. This change broke WASM buildbots.

Example with wasm32-emscripten node (dynamic linking) 3.x: https://buildbot.python.org/all/#/builders/1056/builds/3142

/opt/emsdk/upstream/emscripten/emcc  -DNDEBUG -g -O3 (...) -DPy_BUILD_CORE_BUILTIN -c ../../Modules/_testcapi/vectorcall_limited.c -o Modules/_testcapi/vectorcall_limited.o

In file included from ../../Modules/_testcapi/vectorcall_limited.c:2:
In file included from ../../Modules/_testcapi/parts.h:7:
In file included from ../../Include/Python.h:44:
../../Include/pyport.h:52:4: error: "Py_LIMITED_API is not compatible with Py_BUILD_CORE"
#  error "Py_LIMITED_API is not compatible with Py_BUILD_CORE"
   ^
1 error generated.

I'm not sure how -DPy_BUILD_CORE_BUILTIN landed in the command building Modules/_testcapi/vectorcall_limited.c.

I don't think that it's correct that vectorcall_limited.c which tests the limited C API on purpose it built with Py_BUILD_CORE.

On my Linux machine, Makefile contains:

Modules/_testcapi/vectorcall_limited.o: $(srcdir)/Modules/_testcapi/vectorcall_limited.c $(MODULE__TESTCAPI_DEPS) $(MODULE_DEPS_SHARED) $(PYTHON_HEADERS); $(CC) $(MODULE__TESTCAPI_CFLAGS) $(PY_STDMODULE_CFLAGS) $(CCSHARED) -c $(srcdir)/Modules/_testcapi/vectorcall_limited.c -o Modules/_testcapi/vectorcall_limited.o

So it gets two groups of compiler flags:

  • MODULE__TESTCAPI_CFLAGS: not defined (empty)
  • PY_STDMODULE_CFLAGS: -fno-strict-overflow -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g -Og -Wall -O0 -std=c11 -Werror=implicit-function-declaration -fvisibility=hidden -I./Include/internal -I. -I./Include

I don't see -DPy_BUILD_CORE_BUILTIN here.

@vstinner vstinner mentioned this pull request Sep 22, 2023
Closed
@vstinner

vstinner commented Sep 22, 2023

Copy link
Copy Markdown
Member Author

Oh. This change broke WASM buildbots.

I created issue #109723 to track this issue.

csm10495 pushed a commit to csm10495/cpython that referenced this pull request Sep 28, 2023
@vstinner @csm10495
pythongh-104469: Disallow using Py_LIMITED_API with Py_BUILD_CORE (py… …
c91677e 
…thon#109690)

Fix make check-c-globals: complete USE_LIMITED_C_API list of the
c-analyzer.
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@vstinner @Glyphack
pythongh-104469: Disallow using Py_LIMITED_API with Py_BUILD_CORE (py… …
06c6027 
…thon#109690)

Fix make check-c-globals: complete USE_LIMITED_C_API list of the
c-analyzer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@colesbury colesbury colesbury left review comments

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vstinner @bedevere-bot @erlend-aasland @colesbury