GeezerVison

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

GeezerVison is showing github.com — gh-81057: Move Signal-Related Globals to _PyRuntimeState by ericsnowcurrently · Pull Request #100085 · python/cpython

Conversation

ericsnowcurrently commented Dec 7, 2022 •

edited by bedevere-bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Member

Issue: [subinterpreters] Global C variables are a problem #81057

Sorry, something went wrong.

Uh oh!

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

All reactions

ericsnowcurrently added the skip news label

Dec 7, 2022

netlify Bot commented Dec 7, 2022 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

✅ Deploy Preview for python-cpython-preview canceled.

Name	Link
🔨 Latest commit	`38596a0`
🔍 Latest deploy log	https://app.netlify.com/sites/python-cpython-preview/deploys/6397b5e1a4ac130008314bbc

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-bot mentioned this pull request

Dec 7, 2022

[subinterpreters] Global C variables are a problem #81057

Closed

bedevere-bot added the awaiting core review label

Dec 7, 2022

ericsnowcurrently force-pushed the consolidate-signal-globals branch 2 times, most recently from b8ae626 to 3b416b1 Compare

December 8, 2022 23:45

ericsnowcurrently requested a review from a team as a code owner

December 9, 2022 00:15

ericsnowcurrently force-pushed the consolidate-signal-globals branch 2 times, most recently from 5cd67d8 to c1ac129 Compare

December 9, 2022 17:04

ericsnowcurrently commented

Dec 12, 2022

View reviewed changes

Comment thread

Include/internal/pycore_signal.h Outdated

Comment on lines +15 to +24

		#ifdef MS_WINDOWS
		# ifdef PYCORE_SIGNAL_WITH_PRE_INCLUDES
		# ifndef SOCKET
		# error "<winsock2.h> must be included before this header"
		# endif
		# ifndef HANDLE
		# error "<windows.h> must be included before this header"
		# endif
		# endif
		#endif

ericsnowcurrently Dec 12, 2022

Copy link

Copy Markdown

Member 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.

@zooba, did I do this right? This is how I understood your recommendation.

Sorry, something went wrong.

Uh oh!

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

zooba reacted with thumbs up emoji

All reactions

👍 1 reaction

zooba Dec 12, 2022

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 don't think you want PYCORE_SIGNAL_WITH_PRE_INCLUDES in there. The header file is simply wrong if the system headers haven't been included, so it should always fail.

If the header needs to be used somewhere else, then that somewhere else also needs to include the headers. Alternatively, if it doesn't need to use the values (but just be able to allocate the structures), replace the SOCKET and HANDLE types entirely with void * and do casts anywhere they're actually being used.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Include/internal/pycore_signal.h Resolved

Uh oh!

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

Comment thread

Include/internal/pycore_signal.h Resolved

Uh oh!

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

Comment thread

Include/internal/pycore_signal.h Resolved

Uh oh!

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

Comment thread

Modules/signalmodule.c Resolved

Uh oh!

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

ericsnowcurrently added 17 commits

December 12, 2022 10:00


          Add struct _signals_runtime_state.

6dcffac


          Move signalmodule.c:Handlers to _PyRuntimeState.

5e49464


          Move signalmodule.c:wakeup to _PyRuntimeState.

07332af


          Move signalmodule.c:is_tripped to _PyRuntimeState.

1b5ec60


          Move signalmodule.c:signal_global_state to _PyRuntimeState.

280f6f4


          Fix includes (e.g. for Windows) in pycore_signal.h.

fd6a2de


          Drop the socketmodule.h include.

410190b


          Fix the include for SOCKET on Windows.

1fc3dce


          Fix formatting.

d4f05e4


          Fix order of includes on WINDOWS.

1ddece1


          Fix includes on Windows.

9592a8b


          Fix INVALID_FD on Windows.

cc43787


          Revert "Fix includes on Windows."

This reverts commit d9a5dfa.


          Include the headers conditionally.

1d8d9e2


          Fail instead of falling back to includes.

555f727


          Reach for a sweet spot.

57dc4a8


          Pull in HANDLE.

08799ad

ericsnowcurrently force-pushed the consolidate-signal-globals branch from b8ff9df to 08799ad Compare

December 12, 2022 17:00

ericsnowcurrently added 2 commits

December 12, 2022 10:21


          Drop an unnecessary ifdef.

3bad4d6


          Force the ifdef checks for the actual definition of _PyRuntime.

ericsnowcurrently added 2 commits

December 12, 2022 11:23


          Drop the ifdef checks.

2191c97


          Change members/macros to fail (or warn) when used without the proper …

76794fb

…includes.

ericsnowcurrently removed the request for review from a team

December 12, 2022 18:45

ericsnowcurrently added 6 commits

December 12, 2022 11:56


          Make sure the Windows includes happen early enough.

c9aee62


          Move the includes up.

ca33ad7


          Check for _WINSOCKAPI_ instead of SOCKET.

7f35360


          Define a HANDLE macro where needed.

3d23778


          Use void* for the "fd" field.

73e3bc1


          Use void* for the "sigint_event" field.

4195e62

zooba reviewed

Dec 12, 2022

View reviewed changes

Comment thread

Modules/signalmodule.c Resolved

Uh oh!

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

zooba reviewed

Dec 12, 2022

View reviewed changes

Comment thread

Python/pylifecycle.c Resolved

Uh oh!

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

ericsnowcurrently added 3 commits

December 12, 2022 15:00


          Use an int for wakeup.fd, instead of SOCKET.

36cda7b


          Drop the explicit includes.

2a3bdf1


          Fix a warning.

38596a0

ericsnowcurrently merged commit 5eb28bc into python:main

Dec 12, 2022

bedevere-bot removed the awaiting core review label

Dec 12, 2022

ericsnowcurrently deleted the consolidate-signal-globals branch

December 12, 2022 23:50

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

Dec 14, 2022


          Merge branch 'main' into typedocs

53d33a1

* main: (103 commits)
  pythongh-100248: Add missing `ssl_shutdown_timeout` parameter in `asyncio` docs (python#100249)
  Assorted minor fixes for specialization stats. (pythonGH-100219)
  pythongh-100176: venv: Remove redundant compat code for Python <= 3.2 (python#100177)
  pythonGH-100222: Redefine _Py_CODEUNIT as a union to clarify structure of code unit. (pythonGH-100223)
  pythongh-99955: undef ERROR and SUCCESS before redefining (fixes sanitizer warning) (python#100215)
  pythonGH-100206: use versionadded for the addition of sysconfig.get_default_scheme (python#100207)
  pythongh-81057: Move _Py_RefTotal to the "Ignored Globals" List (pythongh-100203)
  pythongh-81057: Move Signal-Related Globals to _PyRuntimeState (pythongh-100085)
  pythongh-81057: Move faulthandler Globals to _PyRuntimeState (pythongh-100152)
  pythongh-81057: Move tracemalloc Globals to _PyRuntimeState (pythongh-100151)
  pythonGH-100143: Improve collecting pystats for parts of runs (pythonGH-100144)
  pythongh-99955: standardize return values of functions in compiler's code-gen (python#100010)
  pythongh-79218: Define `MS_WIN64` macro for Mingw-w64 64bit on Windows (pythonGH-100137)
  Fix: typo (Indention) (pythonGH-99904)
  pythongh-96715 Remove redundant NULL check in `profile_trampoline` function (python#96716)
  pythongh-100176: remove incorrect version compatibility check from argument clinic (python#100190)
  clarify the 4300-digit limit on int-str conversion (python#100175)
  pythongh-70393: Clarify mention of "middle" scope (python#98839)
  pythongh-99688: Fix outdated tests in test_unary (python#99712)
  pythongh-100174: [Enum] Correct PowersOfThree example. (pythonGH-100178)
  ...

Reviewers

zooba

zooba left review comments

Uh oh!

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

Assignees

No one assigned

Labels

skip news

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.

3 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 01402503 — 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

ericsnowcurrently commented Dec 7, 2022 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify Bot commented Dec 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for python-cpython-preview canceled.

Uh oh!

ericsnowcurrently Dec 12, 2022

Choose a reason for hiding this comment

Uh oh!

zooba Dec 12, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ericsnowcurrently commented Dec 7, 2022 •

edited by bedevere-bot

Loading

Uh oh!

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

netlify Bot commented Dec 7, 2022 •

edited

Loading

Uh oh!

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