gh-125828 : Fix 'get_value' missing on [Bounded]Semaphore on multiprocessing MacOSX#125829
gh-125828 : Fix 'get_value' missing on [Bounded]Semaphore on multiprocessing MacOSX#125829YvesDup wants to merge 9 commits intopython:mainfrom
Conversation
picnixz
left a comment
picnixz
left a comment
There was a problem hiding this comment.
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)
| # Specific MacOSX Semaphore | ||
| # | ||
|
|
||
| class _MacOSXSemaphore(SemLock): |
There was a problem hiding this comment.
Wouldn't it be better to handle the SemLock class differently at the C level?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
This needs a NEWS entry :) |
|
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? |
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 |
YvesDup
changed the title
5 participants
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_initrelative to the Resource Tracker manager.