Skip to content
74 changes: 66 additions & 8 deletions Lib/multiprocessing/synchronize.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,21 +125,79 @@ def _make_name():
return '%s-%s' % (process.current_process()._config['semprefix'],
next(SemLock._rand))

if sys.platform == 'darwin':
#
# Specific MacOSX Semaphore
#

class _MacOSXSemaphore(SemLock):
Copy link
Member

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?

Copy link
Contributor Author

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

Copy link
Member

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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.

"""Dedicated class used only to workaround the missing
function 'sem_getvalue', when interpreter runs on MacOSX.
Add a shared counter for each [Bounded]Semaphore in order
to handle internal counter when acquire and release operations
are called.
"""

def __init__(self, kind, value, maxvalue, *, ctx):
if not isinstance(self, Semaphore):
raise TypeError("_MacOSXSemaphore can only be used "
"as base class of Semaphore class")
self._count = ctx.Value('h', value)
SemLock.__init__(self, kind, value, maxvalue, ctx=ctx)

def _acquire(self, *args, **kwargs) -> bool:
if self._semlock.acquire(*args, **kwargs):
with self._count:
self._count.value -= 1
return True
return False

def _release(self):
with self._count:
self._count.value += 1
self._semlock.release()

def _release_bounded(self):
with self._count:
if self._count.value + 1 > self._semlock.maxvalue:
raise ValueError(f"Cannot exceed initial value of"\
f" {self._semlock.maxvalue!a}")
self._release()

def _get_value(self) -> int:
return self._count.value

def _make_methods(self):
super()._make_methods()
self.acquire = self._acquire
if isinstance(self, BoundedSemaphore):
self.release = self._release_bounded
elif isinstance(self, Semaphore):
self.release = self._release
self.get_value = self._get_value

def __setstate__(self, state):
self._count, state = state[-1], state[:-1]
super().__setstate__(state)

def __getstate__(self) -> tuple:
return super().__getstate__() + (self._count,)


_SemClass = _MacOSXSemaphore
else:
_SemClass = SemLock

#
# Semaphore
#

class Semaphore(SemLock):
class Semaphore(_SemClass):

def __init__(self, value=1, *, ctx):
SemLock.__init__(self, SEMAPHORE, value, SEM_VALUE_MAX, ctx=ctx)
_SemClass.__init__(self, SEMAPHORE, value, SEM_VALUE_MAX, ctx=ctx)

def get_value(self):
'''Returns current value of Semaphore.

Raises NotImplementedError on Mac OSX
because of broken sem_getvalue().
'''
return self._semlock._get_value()

def __repr__(self):
Expand All @@ -156,7 +214,7 @@ def __repr__(self):
class BoundedSemaphore(Semaphore):

def __init__(self, value=1, *, ctx):
SemLock.__init__(self, SEMAPHORE, value, value, ctx=ctx)
_SemClass.__init__(self, SEMAPHORE, value, value, ctx=ctx)

def __repr__(self):
try:
Expand Down
24 changes: 20 additions & 4 deletions Lib/test/_test_multiprocessing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1698,10 +1698,26 @@ def test_semaphore(self):
def test_bounded_semaphore(self):
sem = self.BoundedSemaphore(2)
self._test_semaphore(sem)
# Currently fails on OS/X
#if HAVE_GETVALUE:
# self.assertRaises(ValueError, sem.release)
# self.assertReturnsIfImplemented(2, get_value, sem)
self.assertRaises(ValueError, sem.release)
self.assertReturnsIfImplemented(2, get_value, sem)

@unittest.skipIf(sys.platform != 'darwin', 'Darwin only')
def test_detect_macosx_semaphore(self):
if self.TYPE != 'processes':
self.skipTest('test not appropriate for {}'.format(self.TYPE))

sem = self.Semaphore(2)
mro = sem.__class__.mro()
self.assertTrue(any('_MacOSXSemaphore' in cls.__name__ for cls in mro))

@unittest.skipIf(sys.platform != 'darwin', 'Darwin only')
def test_detect_macosx_boundedsemaphore(self):
if self.TYPE != 'processes':
self.skipTest('test not appropriate for {}'.format(self.TYPE))

sem = self.BoundedSemaphore(2)
mro = sem.__class__.mro()
self.assertTrue(any('_MacOSXSemaphore' in cls.__name__ for cls in mro))

def test_timeout(self):
if self.TYPE != 'processes':
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix the not implemented ``get_value`` for :class:`multiprocessing.Semaphore` on MacOSX
by adding a dedicated class in the ``synchronize.py`` file.
Loading