GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-110014: Fix _POSIX_THREADS and _POSIX_SEMAPHORES usage by vstinner · Pull Request #110139 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Sep 30, 2023 •
edited by github-actions Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member
  • pycore_pythread.h is now the central place to make sure that _POSIX_THREADS and _POSIX_SEMAPHORES macros are defined if available.
  • Make sure that pycore_pythread.h is included when _POSIX_THREADS and _POSIX_SEMAPHORES macros are tested.
  • PY_TIMEOUT_MAX is now defined as a constant, since its value depends on _POSIX_THREADS, instead of being defined as a macro.
  • Document the change and give hints how to fix affected code.
  • Issue: GCC warnings on pycore_semaphore.h and Python/parking_lot.c #110014

📚 Documentation preview 📚: https://cpython-previews--110139.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
pythongh-110014: Fix _POSIX_THREADS and _POSIX_SEMAPHORES usage
d1db176 
* pycore_pythread.h is now the central place to make sure that
  _POSIX_THREADS and _POSIX_SEMAPHORES macros are defined if
  available.
* Make sure that pycore_pythread.h is included when _POSIX_THREADS
  and _POSIX_SEMAPHORES macros are tested.
* PY_TIMEOUT_MAX is now defined as a constant, since its value
  depends on _POSIX_THREADS, instead of being defined as a macro.
* Prevent integer overflow in the preprocessor when computing
  PY_TIMEOUT_MAX_VALUE on Windows:
  replace "0xFFFFFFFELL * 1000 < LLONG_MAX"
  with "0xFFFFFFFELL < LLONG_MAX / 1000".
* Document the change and give hints how to fix affected code.
@bedevere-app bedevere-app Bot added the awaiting review label Sep 30, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Sep 30, 2023
GCC warnings on pycore_semaphore.h and Python/parking_lot.c #110014
Closed
@vstinner vstinner mentioned this pull request Sep 30, 2023
gh-110014: Fix inconsistent struct definition in pycore_semaphore.h #110030
Closed
@vstinner vstinner added the skip news label Sep 30, 2023
@vstinner
Add an exception to smelly.py
9d84457
@vstinner

vstinner commented Sep 30, 2023

Copy link
Copy Markdown
Member Author

@encukou: "Oops", I have to add an exception to Tools/build/smelly.py to tolerate the new symbol PY_TIMEOUT_MAX. Before, Python only exported symbols starting with Py, _Py and __Py (macOS). Copy of my comment:

# "Legacy": some old symbols are prefixed by "PY_".
EXCEPTIONS = frozenset({
    'PY_TIMEOUT_MAX',
})

I would prefer to prefix all public symbols with Py and only have private symbols, but at least, now we have tooling to detect when new symbols with different prefix are added ;-)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
Add PY_TIMEOUT_MAX to the stable ABI
736a14a
@vstinner
vstinner requested review from a team and encukou as code owners September 30, 2023 16:13
@vstinner
vstinner merged commit 74e425e into python:main Sep 30, 2023
@vstinner
vstinner deleted the internal_posix_threads branch September 30, 2023 17:25
@bedevere-app bedevere-app Bot removed the awaiting review label Sep 30, 2023
@vstinner

vstinner commented Sep 30, 2023

Copy link
Copy Markdown
Member Author

Oh. I don't know if it's related or not, but I had to write PR gh-110155 to add many <unistd.h>. WASM/WASI builds were badly broken :-(
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
encukou
encukou reviewed Oct 2, 2023
View reviewed changes
Comment thread Misc/stable_abi.toml
added = '3.2'

# Not mentioned in PEP 384, was implemented as a macro in Python <= 3.12
[data.PY_TIMEOUT_MAX]

@encukou encukou Oct 2, 2023

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.

@vstinner Could we instead exclude this undocumented constant from the limited API?

Sorry, something went wrong.

Uh oh!

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

@vstinner vstinner Oct 2, 2023

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.

I checked and apparently, it's safe to remove this constant, so I created PR gh-110217 for that.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@vstinner @Glyphack
pythongh-110014: Fix _POSIX_THREADS and _POSIX_SEMAPHORES usage (pyth…
28f1396 
…on#110139)

* pycore_pythread.h is now the central place to make sure that
  _POSIX_THREADS and _POSIX_SEMAPHORES macros are defined if
  available.
* Make sure that pycore_pythread.h is included when _POSIX_THREADS
  and _POSIX_SEMAPHORES macros are tested.
* PY_TIMEOUT_MAX is now defined as a constant, since its value
  depends on _POSIX_THREADS, instead of being defined as a macro.
* Prevent integer overflow in the preprocessor when computing
  PY_TIMEOUT_MAX_VALUE on Windows:
  replace "0xFFFFFFFELL * 1000 < LLONG_MAX"
  with "0xFFFFFFFELL < LLONG_MAX / 1000".
* Document the change and give hints how to fix affected code.
* Add an exception for PY_TIMEOUT_MAX  name to smelly.py
* Add PY_TIMEOUT_MAX to the stable ABI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukou encukou encukou 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.

2 participants

@vstinner @encukou
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.