From 9377c22974cbc3241854265bddc7cfd3be1760b0 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 30 May 2020 10:48:22 +0300 Subject: [PATCH 1/5] bpo-24391: Better reprs for threading objects. Add reprs for Semaphore, BoundedSemaphore, Event, and Barrier. --- Lib/test/lock_tests.py | 36 +++++++++++++++++++ Lib/threading.py | 25 +++++++++++++ .../2020-05-30-10-48-04.bpo-24391.ZCTnhX.rst | 1 + 3 files changed, 62 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2020-05-30-10-48-04.bpo-24391.ZCTnhX.rst diff --git a/Lib/test/lock_tests.py b/Lib/test/lock_tests.py index d69bcc9496843f3..9c0e7baac436c5a 100644 --- a/Lib/test/lock_tests.py +++ b/Lib/test/lock_tests.py @@ -453,6 +453,12 @@ def test_at_fork_reinit(self): with evt._cond: self.assertFalse(evt._cond.acquire(False)) + def test_repr(self): + evt = self.eventtype() + self.assertRegex(repr(evt), r"<\w+\.Event: unset at .*>") + evt.set() + self.assertRegex(repr(evt), r"<\w+\.Event: set at .*>") + class ConditionTests(BaseTestCase): """ @@ -800,6 +806,15 @@ def test_release_unacquired(self): sem.acquire() sem.release() + def test_repr(self): + sem = self.semtype(3) + self.assertRegex(repr(sem), r"<\w+\.Semaphore: 3 at .*>") + sem.acquire() + self.assertRegex(repr(sem), r"<\w+\.Semaphore: 2 at .*>") + sem.release() + sem.release() + self.assertRegex(repr(sem), r"<\w+\.Semaphore: 4 at .*>") + class BoundedSemaphoreTests(BaseSemaphoreTests): """ @@ -814,6 +829,12 @@ def test_release_unacquired(self): sem.release() self.assertRaises(ValueError, sem.release) + def test_repr(self): + sem = self.semtype(3) + self.assertRegex(repr(sem), r"<\w+\.BoundedSemaphore: 3/3 at .*>") + sem.acquire() + self.assertRegex(repr(sem), r"<\w+\.BoundedSemaphore: 2/3 at .*>") + class BarrierTests(BaseTestCase): """ @@ -1006,3 +1027,18 @@ def test_single_thread(self): b = self.barriertype(1) b.wait() b.wait() + + def test_repr(self): + b = self.barriertype(3) + self.assertRegex(repr(b), r"<\w+\.Barrier: 0/3 at .*>") + def f(): + b.wait(3) + bunch = Bunch(f, 2) + bunch.wait_for_started() + time.sleep(0.2) + self.assertRegex(repr(b), r"<\w+\.Barrier: 2/3 at .*>") + b.wait(3) + bunch.wait_for_finished() + self.assertRegex(repr(b), r"<\w+\.Barrier: 0/3 at .*>") + b.abort() + self.assertRegex(repr(b), r"<\w+\.Barrier: 0/3, broken at .*>") diff --git a/Lib/threading.py b/Lib/threading.py index ab29db77a747a2d..76b293aa9e3e883 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -401,6 +401,12 @@ def __init__(self, value=1): self._cond = Condition(Lock()) self._value = value + def __repr__(self): + return '<%s.%s: %r at %#x>' % ( + self.__class__.__module__, self.__class__.__name__, + self._value, id(self) + ) + def acquire(self, blocking=True, timeout=None): """Acquire a semaphore, decrementing the internal counter by one. @@ -487,6 +493,12 @@ def __init__(self, value=1): Semaphore.__init__(self, value) self._initial_value = value + def __repr__(self): + return '<%s.%s: %r/%r at %#x>' % ( + self.__class__.__module__, self.__class__.__name__, + self._value, self._initial_value, id(self) + ) + def release(self, n=1): """Release a semaphore, incrementing the internal counter by one or more. @@ -522,6 +534,12 @@ def __init__(self): self._cond = Condition(Lock()) self._flag = False + def __repr__(self): + return '<%s.%s: %s at %#x>' % ( + self.__class__.__module__, self.__class__.__name__, + 'set' if self._flag else 'unset', id(self) + ) + def _at_fork_reinit(self): # Private method called by Thread._reset_internal_locks() self._cond._at_fork_reinit() @@ -611,6 +629,13 @@ def __init__(self, parties, action=None, timeout=None): self._state = 0 #0 filling, 1, draining, -1 resetting, -2 broken self._count = 0 + def __repr__(self): + return '<%s.%s: %r/%r%s at %#x>' % ( + self.__class__.__module__, self.__class__.__name__, + self.n_waiting, self.parties, + ', broken' if self.broken else '', id(self) + ) + def wait(self, timeout=None): """Wait for the barrier. diff --git a/Misc/NEWS.d/next/Library/2020-05-30-10-48-04.bpo-24391.ZCTnhX.rst b/Misc/NEWS.d/next/Library/2020-05-30-10-48-04.bpo-24391.ZCTnhX.rst new file mode 100644 index 000000000000000..d9bed9b7459aef2 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-05-30-10-48-04.bpo-24391.ZCTnhX.rst @@ -0,0 +1 @@ +Improved reprs of :mod:`threading` synchronization objects. From c85e491c4b761dd77dbe1c9fadb2f03f9b29f21d Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 23 Sep 2021 16:57:42 +0300 Subject: [PATCH 2/5] Use __qualname__ insted of __name__ --- Lib/threading.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/Lib/threading.py b/Lib/threading.py index bdcb25c546a9b09..f94f612dc7d77a0 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -116,18 +116,18 @@ def __init__(self): self._count = 0 def __repr__(self): + cls = self.__class__ owner = self._owner try: owner = _active[owner].name except KeyError: pass - return "<%s %s.%s object owner=%r count=%d at %s>" % ( + return "<%s %s.%s object owner=%r count=%d at %#x>" % ( "locked" if self._block.locked() else "unlocked", - self.__class__.__module__, - self.__class__.__qualname__, + cls.__module__, cls.__qualname__, owner, self._count, - hex(id(self)) + id(self) ) def _at_fork_reinit(self): @@ -419,8 +419,9 @@ def __init__(self, value=1): self._value = value def __repr__(self): + cls = self.__class__ return '<%s.%s: %r at %#x>' % ( - self.__class__.__module__, self.__class__.__name__, + cls.__module__, cls.__qualname__, self._value, id(self) ) @@ -511,8 +512,9 @@ def __init__(self, value=1): self._initial_value = value def __repr__(self): + cls = self.__class__ return '<%s.%s: %r/%r at %#x>' % ( - self.__class__.__module__, self.__class__.__name__, + cls.__module__, cls.__qualname__, self._value, self._initial_value, id(self) ) @@ -552,8 +554,9 @@ def __init__(self): self._flag = False def __repr__(self): + cls = self.__class__ return '<%s.%s: %s at %#x>' % ( - self.__class__.__module__, self.__class__.__name__, + cls.__module__, cls.__qualname__, 'set' if self._flag else 'unset', id(self) ) @@ -656,8 +659,9 @@ def __init__(self, parties, action=None, timeout=None): self._count = 0 def __repr__(self): + cls = self.__class__ return '<%s.%s: %r/%r%s at %#x>' % ( - self.__class__.__module__, self.__class__.__name__, + cls.__module__, cls.__qualname__, self.n_waiting, self.parties, ', broken' if self.broken else '', id(self) ) From ca388b4a1b680fd1e9aa7b13db9acddf45a9dda5 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 27 Sep 2021 10:25:39 +0300 Subject: [PATCH 3/5] Update a NEWS entry. --- .../next/Library/2020-05-30-10-48-04.bpo-24391.ZCTnhX.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2020-05-30-10-48-04.bpo-24391.ZCTnhX.rst b/Misc/NEWS.d/next/Library/2020-05-30-10-48-04.bpo-24391.ZCTnhX.rst index d9bed9b7459aef2..15add1531501aa4 100644 --- a/Misc/NEWS.d/next/Library/2020-05-30-10-48-04.bpo-24391.ZCTnhX.rst +++ b/Misc/NEWS.d/next/Library/2020-05-30-10-48-04.bpo-24391.ZCTnhX.rst @@ -1 +1,3 @@ -Improved reprs of :mod:`threading` synchronization objects. +Improved reprs of :mod:`threading` synchronization objects: +:class:`~threading.Semaphore`, :class:`~threading.BoundedSemaphore`, +:class:`~threading.Event` and :class:`~threading.Barrier`. From 38e7012ab0733b9500579eecfe60048f65e1d614 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 27 Sep 2021 10:41:15 +0300 Subject: [PATCH 4/5] Rewrite. --- Lib/test/lock_tests.py | 22 +++++++++++----------- Lib/threading.py | 27 ++++++++++----------------- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/Lib/test/lock_tests.py b/Lib/test/lock_tests.py index 6240b52f98e0449..d82629368dff8a5 100644 --- a/Lib/test/lock_tests.py +++ b/Lib/test/lock_tests.py @@ -457,9 +457,9 @@ def test_at_fork_reinit(self): def test_repr(self): evt = self.eventtype() - self.assertRegex(repr(evt), r"<\w+\.Event: unset at .*>") + self.assertRegex(repr(evt), r"<\w+\.Event at .*: unset>") evt.set() - self.assertRegex(repr(evt), r"<\w+\.Event: set at .*>") + self.assertRegex(repr(evt), r"<\w+\.Event at .*: set>") class ConditionTests(BaseTestCase): @@ -810,12 +810,12 @@ def test_release_unacquired(self): def test_repr(self): sem = self.semtype(3) - self.assertRegex(repr(sem), r"<\w+\.Semaphore: 3 at .*>") + self.assertRegex(repr(sem), r"<\w+\.Semaphore at .*: value=3>") sem.acquire() - self.assertRegex(repr(sem), r"<\w+\.Semaphore: 2 at .*>") + self.assertRegex(repr(sem), r"<\w+\.Semaphore at .*: value=2>") sem.release() sem.release() - self.assertRegex(repr(sem), r"<\w+\.Semaphore: 4 at .*>") + self.assertRegex(repr(sem), r"<\w+\.Semaphore at .*: value=4>") class BoundedSemaphoreTests(BaseSemaphoreTests): @@ -833,9 +833,9 @@ def test_release_unacquired(self): def test_repr(self): sem = self.semtype(3) - self.assertRegex(repr(sem), r"<\w+\.BoundedSemaphore: 3/3 at .*>") + self.assertRegex(repr(sem), r"<\w+\.BoundedSemaphore at .*: value=3/3>") sem.acquire() - self.assertRegex(repr(sem), r"<\w+\.BoundedSemaphore: 2/3 at .*>") + self.assertRegex(repr(sem), r"<\w+\.BoundedSemaphore at .*: value=2/3>") class BarrierTests(BaseTestCase): @@ -1032,15 +1032,15 @@ def test_single_thread(self): def test_repr(self): b = self.barriertype(3) - self.assertRegex(repr(b), r"<\w+\.Barrier: 0/3 at .*>") + self.assertRegex(repr(b), r"<\w+\.Barrier at .*: waiters=0/3>") def f(): b.wait(3) bunch = Bunch(f, 2) bunch.wait_for_started() time.sleep(0.2) - self.assertRegex(repr(b), r"<\w+\.Barrier: 2/3 at .*>") + self.assertRegex(repr(b), r"<\w+\.Barrier at .*: waiters=2/3>") b.wait(3) bunch.wait_for_finished() - self.assertRegex(repr(b), r"<\w+\.Barrier: 0/3 at .*>") + self.assertRegex(repr(b), r"<\w+\.Barrier at .*: waiters=0/3>") b.abort() - self.assertRegex(repr(b), r"<\w+\.Barrier: 0/3, broken at .*>") + self.assertRegex(repr(b), r"<\w+\.Barrier at .*: broken>") diff --git a/Lib/threading.py b/Lib/threading.py index f94f612dc7d77a0..704f032f08a2dc5 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -420,10 +420,8 @@ def __init__(self, value=1): def __repr__(self): cls = self.__class__ - return '<%s.%s: %r at %#x>' % ( - cls.__module__, cls.__qualname__, - self._value, id(self) - ) + return (f"<{cls.__module__}.{cls.__qualname__} at {id(self):#x}:" + f" value={self._value}>") def acquire(self, blocking=True, timeout=None): """Acquire a semaphore, decrementing the internal counter by one. @@ -513,10 +511,8 @@ def __init__(self, value=1): def __repr__(self): cls = self.__class__ - return '<%s.%s: %r/%r at %#x>' % ( - cls.__module__, cls.__qualname__, - self._value, self._initial_value, id(self) - ) + return (f"<{cls.__module__}.{cls.__qualname__} at {id(self):#x}:" + f" value={self._value}/{self._initial_value}>") def release(self, n=1): """Release a semaphore, incrementing the internal counter by one or more. @@ -555,10 +551,8 @@ def __init__(self): def __repr__(self): cls = self.__class__ - return '<%s.%s: %s at %#x>' % ( - cls.__module__, cls.__qualname__, - 'set' if self._flag else 'unset', id(self) - ) + status = 'set' if self._flag else 'unset' + return f"<{cls.__module__}.{cls.__qualname__} at {id(self):#x}: {status}>" def _at_fork_reinit(self): # Private method called by Thread._reset_internal_locks() @@ -660,11 +654,10 @@ def __init__(self, parties, action=None, timeout=None): def __repr__(self): cls = self.__class__ - return '<%s.%s: %r/%r%s at %#x>' % ( - cls.__module__, cls.__qualname__, - self.n_waiting, self.parties, - ', broken' if self.broken else '', id(self) - ) + if self.broken: + return f"<{cls.__module__}.{cls.__qualname__} at {id(self):#x}: broken>" + return (f"<{cls.__module__}.{cls.__qualname__} at {id(self):#x}:" + f" waiters={self.n_waiting}/{self.parties}>") def wait(self, timeout=None): """Wait for the barrier. From 62fcc2ae864fceb7c49c72474d5b7fab98966214 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 27 Sep 2021 10:46:14 +0300 Subject: [PATCH 5/5] Refactor. --- Lib/threading.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/threading.py b/Lib/threading.py index 704f032f08a2dc5..e98edca72d5c05d 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -116,18 +116,18 @@ def __init__(self): self._count = 0 def __repr__(self): - cls = self.__class__ owner = self._owner try: owner = _active[owner].name except KeyError: pass - return "<%s %s.%s object owner=%r count=%d at %#x>" % ( + return "<%s %s.%s object owner=%r count=%d at %s>" % ( "locked" if self._block.locked() else "unlocked", - cls.__module__, cls.__qualname__, + self.__class__.__module__, + self.__class__.__qualname__, owner, self._count, - id(self) + hex(id(self)) ) def _at_fork_reinit(self):