Merge pull request #339 from njsmith/no-slots-sigh

Stop using slots=True in public classes to fix weakref issues

trio/_core/_parking_lot.py

@@ -86,7 +86,7 @@ class _ParkingLotStatistics:
     tasks_waiting = attr.ib()
 
 
-@attr.s(slots=True, cmp=False, hash=False)
+@attr.s(cmp=False, hash=False)
 class ParkingLot:
     """A fair wait queue with cancellation and requeueing.
 

trio/_core/_result.py

@@ -4,7 +4,6 @@
 __all__ = ["Result", "Value", "Error"]
 
 
-@attr.s(slots=True, frozen=True)
 class Result(metaclass=abc.ABCMeta):
     """An abstract class representing the result of a Python computation.
 
@@ -20,6 +19,7 @@ class Result(metaclass=abc.ABCMeta):
     hashable.
 
     """
+    __slots__ = ()
 
     @staticmethod
     def capture(sync_fn, *args):
@@ -81,7 +81,7 @@ async def asend(self, agen):
         """
 
 
-@attr.s(slots=True, frozen=True, repr=False)
+@attr.s(frozen=True, repr=False)
 class Value(Result):
     """Concrete :class:`Result` subclass representing a regular value.
 
@@ -103,7 +103,7 @@ async def asend(self, agen):
         return await agen.asend(self.value)
 
 
-@attr.s(slots=True, frozen=True, repr=False)
+@attr.s(frozen=True, repr=False)
 class Error(Result):
     """Concrete :class:`Result` subclass representing a raised exception.
 

trio/_core/_run.py

@@ -62,7 +62,7 @@
 _r = random.Random()
 
 
-@attr.s(slots=True, frozen=True)
+@attr.s(frozen=True)
 class SystemClock:
     # Add a large random offset to our clock to ensure that if people
     # accidentally call time.monotonic() directly or start comparing clocks
@@ -84,7 +84,7 @@ def deadline_to_sleep_time(self, deadline):
 ################################################################
 
 
-@attr.s(slots=True, cmp=False, hash=False)
+@attr.s(cmp=False, hash=False)
 class CancelScope:
     _tasks = attr.ib(default=attr.Factory(set))
     _effective_deadline = attr.ib(default=inf)
@@ -205,7 +205,7 @@ def open_cancel_scope(*, deadline=inf, shield=False):
 
 # This code needs to be read alongside the code from Nursery.start to make
 # sense.
-@attr.s(slots=True, cmp=False, hash=False, repr=False)
+@attr.s(cmp=False, hash=False, repr=False)
 class _TaskStatus:
     _old_nursery = attr.ib()
     _new_nursery = attr.ib()
@@ -532,7 +532,7 @@ def _pending_cancel_scope(cancel_stack):
     return pending_scope
 
 
-@attr.s(slots=True, cmp=False, hash=False, repr=False)
+@attr.s(cmp=False, hash=False, repr=False)
 class Task:
     _parent_nursery = attr.ib()
     coro = attr.ib()

trio/_core/tests/test_result.py

@@ -10,15 +10,13 @@ def test_Result():
     v = Value(1)
     assert v.value == 1
     assert v.unwrap() == 1
-    assert not hasattr(v, "__dict__")
     assert repr(v) == "Value(1)"
 
     exc = RuntimeError("oops")
     e = Error(exc)
     assert e.error is exc
     with pytest.raises(RuntimeError):
         e.unwrap()
-    assert not hasattr(e, "__dict__")
     assert repr(e) == "Error(RuntimeError('oops',))"
 
     with pytest.raises(TypeError):

trio/_core/tests/test_run.py

@@ -421,7 +421,7 @@ async def child():
     assert stats.seconds_to_next_deadline == inf
 
 
-@attr.s(slots=True, cmp=False, hash=False)
+@attr.s(cmp=False, hash=False)
 class TaskRecorder:
     record = attr.ib(default=attr.Factory(list))
 

trio/_highlevel_generic.py

@@ -94,7 +94,7 @@ class ClosedListenerError(Exception):
     pass
 
 
-@attr.s(slots=True, cmp=False, hash=False)
+@attr.s(cmp=False, hash=False)
 class StapledStream(HalfCloseableStream):
     """This class `staples <https://en.wikipedia.org/wiki/Staple_(fastener)>`__
     together two unidirectional streams to make single bidirectional stream.

trio/_sync.py

@@ -18,7 +18,7 @@
 ]
 
 
-@attr.s(slots=True, repr=False, cmp=False, hash=False)
+@attr.s(repr=False, cmp=False, hash=False)
 class Event:
     """A waitable boolean value useful for inter-task synchronization,
     inspired by :class:`threading.Event`.
@@ -486,7 +486,7 @@ class _LockStatistics:
 
 
 @async_cm
-@attr.s(slots=True, cmp=False, hash=False, repr=False)
+@attr.s(cmp=False, hash=False, repr=False)
 class Lock:
     """A classic `mutex
     <https://en.wikipedia.org/wiki/Lock_(computer_science)>`__.

trio/_threads.py

@@ -259,7 +259,7 @@ def current_default_worker_thread_limiter():
 # system; see https://github.com/python-trio/trio/issues/182
 # But for now we just need an object to stand in for the thread, so we can
 # keep track of who's holding the CapacityLimiter's token.
-@attr.s(frozen=True, cmp=False, hash=False, slots=True)
+@attr.s(frozen=True, cmp=False, hash=False)
 class ThreadPlaceholder:
     name = attr.ib()
 

trio/testing/_sequencer.py

@@ -10,7 +10,7 @@
 __all__ = ["Sequencer"]
 
 
-@attr.s(slots=True, cmp=False, hash=False)
+@attr.s(cmp=False, hash=False)
 class Sequencer:
     """A convenience class for forcing code in different tasks to run in an
     explicit linear order.

trio/tests/test_sync.py

@@ -1,5 +1,7 @@
 import pytest
 
+import weakref
+
 from ..testing import wait_all_tasks_blocked, assert_checkpoints
 
 from .. import _core
@@ -215,6 +217,11 @@ async def test_Semaphore_bounded():
 async def test_Lock_and_StrictFIFOLock(lockcls):
     l = lockcls()
     assert not l.locked()
+
+    # make sure locks can be weakref'ed (gh-331)
+    r = weakref.ref(l)
+    assert r() is l
+
     repr(l)  # smoke test
     # make sure repr uses the right name for subclasses
     assert lockcls.__name__ in repr(l)