Breaking circular reference between Actor and its ActorRef

[JayPay108]

Fixes #249

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.32%. Comparing base (4168837) to head (3c6146c).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #250   +/-   ##
=======================================
  Coverage   94.32%   94.32%           
=======================================
  Files          14       14           
  Lines         581      582    +1     
  Branches       50       50           
=======================================
+ Hits          548      549    +1     
  Misses         29       29           
  Partials        4        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jodal]

I find it a bit icky to let Actor dig deep into ActorRef internals to remove a reference to itself, so I wanted to see if there's other ways to do this.

I looked at if ActorRef._actor could be removed entirely, and found that we only use it for the proxy introspection when creating an ActorProxy from an ActorRef. The ActorProxy introspection only happens after a check that the actor is actually alive, which explain why it works to unset this field without breaking any tests.

Another approach here would be to make ActorRef's reference to Actor a "weakref", so that ActorRef isn't contributing to keeping the Actor object alive.

This patch seems to pass the test suite, but does it solve your problem? Can you test if the actor is properly garbage collected with this patch?

diff --git i/src/pykka/_proxy.py w/src/pykka/_proxy.py
index 3371704..0c06fed 100644
--- i/src/pykka/_proxy.py
+++ w/src/pykka/_proxy.py
@@ -139,7 +139,10 @@ class ActorProxy(Generic[A]):
             msg = f"{actor_ref} not found"
             raise ActorDeadError(msg)
         self.actor_ref = actor_ref
-        self._actor = actor_ref._actor  # noqa: SLF001
+        self._actor = actor_ref._actor_weakref()  # noqa: SLF001
+        if self._actor is None:
+            msg = f"{actor_ref} not found (weakref has been deallocated)"
+            raise ActorDeadError(msg)
         self._attr_path = attr_path or ()
         self._known_attrs = introspect_attrs(root=self._actor, proxy=self)
         self._actor_proxies = {}
diff --git i/src/pykka/_ref.py w/src/pykka/_ref.py
index 7ca8677..4eadc63 100644
--- i/src/pykka/_ref.py
+++ w/src/pykka/_ref.py
@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import weakref
 from typing import (
     TYPE_CHECKING,
     Any,
@@ -51,7 +52,7 @@ class ActorRef(Generic[A]):
         self,
         actor: A,
     ) -> None:
-        self._actor = actor
+        self._actor_weakref = weakref.ref(actor)
         self.actor_class = actor.__class__
         self.actor_urn = actor.actor_urn
         self.actor_inbox = actor.actor_inbox

[JayPay108]

@jodal Super fair. Looks like your patch works great. A stopped actor is garbage collected instantly after removing the last external reference. Thanks!

[jodal]

Thanks for testing, I'll get this into a release this weekend 😄

[jodal]

v4.4.2 with the fix is out now!