Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1733 #1734

Merged
merged 2 commits into from
Dec 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 7 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,14 @@ jobs:
name: gevent-${{ runner.os }}-${{ matrix.python-version }}.whl
path: dist/*whl
- name: Publish package to PyPI (mac)
uses: pypa/gh-action-pypi-publish@v1.4.1
# We cannot 'uses: pypa/gh-action-pypi-publish@v1.4.1' because
# that's apparently a container action, and those don't run on
# the Mac.
if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags') && startsWith(runner.os, 'Mac')
with:
user: __token__
password: ${{ secrets.TWINE_PASSWORD }}
skip_existing: true
env:
TWINE_PASSWORD: ${{ secrets.TWINE_PASSWORD }}
run: |
twine upload --skip-existing dist/*

- name: Install gevent
# I'd prefer to install the wheel in non-editable mode, but that seems to
Expand Down
5 changes: 5 additions & 0 deletions docs/changes/1733.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Make gevent's ``Semaphore`` objects properly handle native thread
identifiers larger than can be stored in a C ``long`` on Python 3,
instead of raising an ``OverflowError``.

Reported by TheYOSH.
13 changes: 12 additions & 1 deletion src/gevent/_gevent_c_semaphore.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,25 @@ cdef _native_sleep
cdef monotonic
cdef spawn_raw

cdef _UNSET
cdef _MULTI

cdef class _LockReleaseLink(object):
cdef object lock


cdef class Semaphore(AbstractLinkable):
cdef public int counter
# On Python 3, thread.get_ident() returns a ``unsigned long``; on
# Python 2, it's a plain ``long``. We can conditionally change
# the type here (depending on which version is cythonizing the
# .py files), but: our algorithm for testing whether it has been
# set or not was initially written with ``long`` in mind and used
# -1 as a sentinel. That doesn't work on Python 3. Thus, we can't
# use the native C type and must keep the wrapped Python object, which
# we can test for None.
cdef object _multithreaded

cdef long _multithreaded

cpdef bint locked(self)
cpdef int release(self) except -1000
Expand Down
19 changes: 12 additions & 7 deletions src/gevent/_semaphore.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ def __init__(self, lock):
def __call__(self, _):
self.lock.release()

_UNSET = object()
_MULTI = object()

class Semaphore(AbstractLinkable): # pylint:disable=undefined-variable
"""
Semaphore(value=1) -> Semaphore
Expand Down Expand Up @@ -78,9 +81,11 @@ class Semaphore(AbstractLinkable): # pylint:disable=undefined-variable

__slots__ = (
'counter',
# Integer. Set to 0 initially. Set to the ident of the first
# thread that acquires us. If we later see a different thread
# ident, set to -1.
# long integer, signed (Py2) or unsigned (Py3); see comments
# in the .pxd file for why we store as Python object. Set to ``_UNSET``
# initially. Set to the ident of the first thread that
# acquires us. If we later see a different thread ident, set
# to ``_MULTI``.
'_multithreaded',
)

Expand All @@ -90,7 +95,7 @@ def __init__(self, value=1, hub=None):
raise ValueError("semaphore initial value must be >= 0")
super(Semaphore, self).__init__(hub)
self._notify_all = False
self._multithreaded = 0
self._multithreaded = _UNSET

def __str__(self):
return '<%s at 0x%x counter=%s _links[%s]>' % (
Expand Down Expand Up @@ -196,10 +201,10 @@ def acquire(self, blocking=True, timeout=None):
"""
# pylint:disable=too-many-return-statements,too-many-branches
# Sadly, the body of this method is rather complicated.
if self._multithreaded == 0:
if self._multithreaded is _UNSET:
self._multithreaded = self._get_thread_ident()
elif self._multithreaded != self._get_thread_ident():
self._multithreaded = -1
self._multithreaded = _MULTI

# We conceptually now belong to the hub of the thread that
# called this, whether or not we have to block. Note that we
Expand All @@ -225,7 +230,7 @@ def acquire(self, blocking=True, timeout=None):
if not blocking:
return False

if self._multithreaded != -1 and self.hub is None: # pylint:disable=access-member-before-definition
if self._multithreaded is not _MULTI and self.hub is None: # pylint:disable=access-member-before-definition
self.hub = get_hub() # pylint:disable=attribute-defined-outside-init

if self.hub is None and not invalid_thread_use:
Expand Down