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-125828 : Fix 'get_value' missing on [Bounded]Semaphore on multiprocessing MacOSX by YvesDup · Pull Request #125829 · python/cpython

Conversation

@YvesDup

@YvesDup YvesDup commented Oct 22, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

This proposal is a workaround to fix absence of 'sem_getvalue' C function in the Semaphore MacOSX implementation.

Alls unit tests succeed except on test.test_concurrent_futures.test_init relative to the Resource Tracker manager.

  • Issue: Multiprocessing Semaphore and BoundedSemaphore - incomplete features ('get_value' missing) on MacOSX. #125828

Sorry, something went wrong.

Uh oh!

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

All reactions
@YvesDup
Initial commit
5d2997e
@YvesDup
YvesDup requested a review from gpshead as a code owner October 22, 2024 07:13
@bedevere-app bedevere-app Bot added the awaiting review label Oct 22, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Oct 22, 2024
Multiprocessing Semaphore and BoundedSemaphore - incomplete features ('get_value' missing) on MacOSX. #125828
Open
picnixz
picnixz reviewed Oct 22, 2024
View reviewed changes

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

Is there an alternative way to do it without a temporary class like this? (I don't have the answer but I feel it overcomplicates the code base)

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/multiprocessing/synchronize.py
# Specific MacOSX Semaphore
#

class _MacOSXSemaphore(SemLock):

@picnixz picnixz Oct 22, 2024

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.

Wouldn't it be better to handle the SemLock class differently at the C level?

Sorry, something went wrong.

Uh oh!

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

All reactions

@YvesDup YvesDup Oct 22, 2024

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.

Wouldn't it be better to handle the SemLock class differently at the C level?

As I mentioned in the problem, I'm waiting for feedback (from the Mac OS team ?) before going any further.

A C implementation is an option, even if it seems more complicated than Python.
That said, I'm available to go deep into the SemLock C code

Sorry, something went wrong.

Uh oh!

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

All reactions

@picnixz picnixz Oct 22, 2024

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.

Actually, since the SemLock class is implemented in C, it's preferrable to first patch it there IMO. But let's CC some people I know they are on macOS: @ned-deily @ronaldoussoren @freakboy3742.

Sorry, something went wrong.

Uh oh!

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

All reactions

@freakboy3742 freakboy3742 Oct 23, 2024

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.

I can't profess any particular expertise in this area. However, given the underlying implementation is written in C, I agree that it makes some sense for the implementation of this workaround to also be in C (rather than have an underlying implementation that returns NotImplemented, wrapped by a layer that overrides that.

Sorry, something went wrong.

Uh oh!

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

All reactions

@YvesDup YvesDup Nov 4, 2024

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.

rather than have an underlying implementation that returns NotImplemented, wrapped by a layer that overrides that.

The main idea is to listen to the acquire and release methods to update a shared counter, and to return the counter value when the get_value method is invoked.

I am going to open a new PR with a workaround written in C.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/multiprocessing/synchronize.py Resolved

Uh oh!

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

Comment thread Lib/multiprocessing/synchronize.py Resolved

Uh oh!

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

Comment thread Lib/multiprocessing/synchronize.py Resolved

Uh oh!

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

@ZeroIntensity

ZeroIntensity commented Oct 23, 2024

Copy link
Copy Markdown
Member

This needs a NEWS entry :)

YvesDup reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

@ronaldoussoren

ronaldoussoren commented Oct 25, 2024

Copy link
Copy Markdown
Contributor

Is get_value part of the public API for these classes? The name suggests it is, but the method is not document in the reference docs and does not have a docsstring.

I haven't looked with enough detail at the code to be certain, and am not a multiprocessing expert, but I the code doesn't seem correct. In particular, how does the implementation handle blocking callers of acquire when the count is 0 but not when it is larger than 0?

All reactions

Sorry, something went wrong.

Uh oh!

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

@YvesDup

YvesDup commented Nov 4, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

is get_value part of the public API for these classes? The name suggests it is, but the method is not document in the reference docs and does not have a docsstring.

Yes, it is and I confirm this is not documented. This public method is rarely used to get a value of semaphore, except in unit tests. I should open an issue about this case.

In particular, how does the implementation handle blocking callers of acquire when the count is 0 but not when it is larger than 0?

As I explain above, this class only listen to acquire and release to update shared counter and then call respectively acquire and releasemethods of _semlock class. When get_value is invoked, the class returns the counter value, and never calls the semlock._get_value method (which raises NotImplementedError).

In this way, the behaviour of semaphores is always done at a low level in the implementation of the semlockclass in the semaphore module (at C level).

All reactions

Sorry, something went wrong.

Uh oh!

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

@YvesDup YvesDup changed the title gh-125828 : Fix 'get_value' missing on [Bouded]Semaphore on multiprocessing MacOSX gh-125828 : Fix 'get_value' missing on [Bounded]Semaphore on multiprocessing MacOSX Nov 4, 2024
@YvesDup YvesDup mentioned this pull request Mar 6, 2025
gh-125828 : Fix 'get_value' missing for [Bounded]Semaphore on multiprocessing MacOSX - C workaround #130913
Open
@YvesDup
Merge branch 'python:main' into semaphore-macosx-multiprocessing
3c9dda3
@YvesDup YvesDup closed this Apr 11, 2025
@YvesDup
YvesDup deleted the semaphore-macosx-multiprocessing branch April 11, 2025 13:47
@YvesDup
YvesDup restored the semaphore-macosx-multiprocessing branch April 11, 2025 13:47
@YvesDup YvesDup reopened this Apr 13, 2025
@YvesDup YvesDup mentioned this pull request Apr 29, 2025
Document the public multiprocessing.[Bounded]Semaphore.get_value method and refactor code #133146
Closed
YvesDup and others added 10 commits May 3, 2025 10:37
@YvesDup
Merge branch 'main' into semaphore-macosx-multiprocessing
5aeff96
@YvesDup
Merge branch 'python:main' into semaphore-macosx-multiprocessing
171cc78
@YvesDup
Merge branch 'main' into semaphore-macosx-multiprocessing
23d7737
@YvesDup
Merge branch 'main' into semaphore-macosx-multiprocessing
9eddb4c
@YvesDup
Merge branch 'python:main' into semaphore-macosx-multiprocessing
adf0a01
@blurb-it
📜🤖 Added by blurb_it.
6e2c56c
@YvesDup
Refactor code
5472ce2
@YvesDup
Merge branch 'python:main' into semaphore-macosx-multiprocessing
0abca1c
@YvesDup
Semaphore macosx multiprocessing fix (#6)
a486952
* Refactor code
* fix error on get_value, and nits
@YvesDup
last modifications as:
e69bfd8
+ Add `pending_acquires` counter
+ Update comments
+ Fix nits
@read-the-docs-community

read-the-docs-community Bot commented May 7, 2026

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #32588791 | 📁 Comparing e69bfd8 against main (299cf3c)

  🔍 Preview build  

194 files changed · ± 191 modified · - 3 deleted

± Modified

  • glossary.html
  • index.html
  • c-api/allocation.html
  • c-api/buffer.html
  • c-api/bytearray.html
  • c-api/bytes.html
  • c-api/capsule.html
  • c-api/code.html
  • c-api/concrete.html
  • c-api/descriptor.html
  • and 181 more...

- Deleted

  • c-api/sentinel.html
  • c-api/slots.html
  • deprecations/soft-deprecations.html
All reactions

Sorry, something went wrong.

Uh oh!

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

YvesDup added 3 commits May 7, 2026 18:45
@YvesDup
Merge branch 'main' into semaphore-macosx-multiprocessing
73e9a5b
@YvesDup
Merge branch 'main' into semaphore-macosx-multiprocessing
7a0fd46
@YvesDup
Update news file
ae275bf
Clarified the fix for the unimplemented get_value method and specified the class hierarchy for better understanding.
@YvesDup
Update following the execution of “regen-all”
7080b17
@YvesDup
YvesDup requested a review from ericsnowcurrently as a code owner May 10, 2026 16:44
YvesDup added 6 commits May 10, 2026 19:08
@YvesDup
Fix bad class name
7fab36b
@YvesDup
Merge branch 'main' into semaphore-macosx-multiprocessing
5d8b72c
@YvesDup
revert update of ./Programs/test_frozenmain.h
d07a8d4
@YvesDup
Merge branch 'main' into semaphore-macosx-multiprocessing
5fb2532
@YvesDup
Merge branch 'main' into semaphore-macosx-multiprocessing
79e1ac5
@YvesDup
Merge branch 'main' into semaphore-macosx-multiprocessing
405d3fd
@github-actions

github-actions Bot commented Oct 30, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 90 days with no activity.

All reactions

Sorry, something went wrong.

Uh oh!

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

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Oct 30, 2026
@YvesDup
Merge branch 'main' into semaphore-macosx-multiprocessing
4e0463c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@freakboy3742 freakboy3742 freakboy3742 left review comments
@picnixz picnixz picnixz left review comments
@gpshead gpshead Awaiting requested review from gpshead gpshead is a code owner
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently

Uh oh!

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

Assignees

No one assigned

Labels

awaiting review stale Stale PR or inactive for long period of time.

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.

5 participants

@YvesDup @ZeroIntensity @ronaldoussoren @freakboy3742 @picnixz
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.