GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||
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
|
| # Specific MacOSX Semaphore | ||
| # | ||
|
|
||
| class _MacOSXSemaphore(SemLock): |
There was a problem 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.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
This needs a NEWS entry :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Is 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? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
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.
As I explain above, this class only listen to In this way, the behaviour of semaphores is always done at a low level in the implementation of the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
* Refactor code * fix error on get_value, and nits
+ Add `pending_acquires` counter + Update comments + Fix nits
Documentation build overview
194 files changed ·
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Clarified the fix for the unimplemented get_value method and specified the class hierarchy for better understanding.
|
This PR is stale because it has been open for 90 days with no activity. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.