made `ActorCell` use `nullable` #7475

Aaronontheweb · 2025-01-20T22:06:29Z

Changes

This PR looks scarier than it is. I've implemented nullability on the ActorCell, the brains underneath each actor, in order to support some future changes we're going to make here. None of this should result in a behavior change or a breaking API change - practically speaking, given that most of these APIs are not called directly from user code.

Checklist

For significant changes, please ensure that the following have been completed (delete if not relevant):

This change follows the Akka.NET API Compatibility Guidelines.
This change follows the Akka.NET Wire Compatibility Guidelines.
I have reviewed my own pull request.

Aaronontheweb · 2025-01-20T22:07:13Z

I've also cleaned up A TON of TBD stuff in the XML-DOCS while I was at it. Didn't get everything, but got a lot of it.

Aaronontheweb

Detailed my changes

Aaronontheweb · 2025-01-20T22:07:45Z

src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCore.DotNet.verified.txt

API approval file - you should still look over it but there's nothing super scary here. Going to comment on the changes in the source files themselves.

Aaronontheweb · 2025-01-20T22:09:38Z

src/core/Akka/Actor/ActorCell.Children.cs

@@ -36,10 +36,10 @@ private IReadOnlyCollection<IActorRef> Children
        }

        private ImmutableDictionary<string, FunctionRef> FunctionRefs => Volatile.Read(ref _functionRefsDoNotCallMeDirectly);
-        internal bool TryGetFunctionRef(string name, out FunctionRef functionRef) =>
+        internal bool TryGetFunctionRef(string name, out FunctionRef? functionRef) =>


All of these TryGetValue<TKey, TValue> methods have a signature of bool TryGetValue<TKey, TValue>(TKey key, out TValue? val)in the .NET standard libraries - so I've brought theTryGetFunctionRef` up to speed.

Aaronontheweb · 2025-01-20T22:10:39Z

src/core/Akka/Actor/ActorCell.Children.cs

So a really useful over-arching point for all of these code changes in the review:

We're not changing what is actually returned - we're just accurately annotating it going forward.

Please bear that in mind - only input behavior has changed (i.e. explicitly handling null inputs where we weren't before), not any output behavior. Methods that could return null before could still return null, but it's just annotated now.

Aaronontheweb · 2025-01-20T22:14:49Z

src/core/Akka/Actor/ActorCell.Children.cs

@@ -199,7 +199,7 @@ protected void UnreserveChild(string name)
        /// </summary>
        /// <param name="actor">TBD</param>
        /// <returns>TBD</returns>
-        public ChildRestartStats InitChild(IInternalActorRef actor)
+        public ChildRestartStats? InitChild(IInternalActorRef actor)


ChildRestartStats will be null if we don't have a child by this name - should never happen but it has always been theoretically possible

Aaronontheweb · 2025-01-20T22:15:20Z

src/core/Akka/Actor/ActorCell.Children.cs

@@ -318,7 +318,7 @@ public bool TryGetChildStatsByName(string name, out IChildStats child)   //This
        /// <summary>
        /// Tries to get the stats for the child with the specified name. This ignores children for whom only names have been reserved.
        /// </summary>
-        private bool TryGetChildRestartStatsByName(string name, out ChildRestartStats child)
+        private bool TryGetChildRestartStatsByName(string name, out ChildRestartStats? child)


Again, the TryGetValue pattern applied here

Aaronontheweb · 2025-01-20T22:31:12Z

src/core/Akka/Actor/ActorCell.cs

        /// </summary>
-        public int NumberOfMessages { get { return Mailbox.NumberOfMessages; } }
+        public int NumberOfMessages { get { return Mailbox!.NumberOfMessages; } }


Mailbox can't be null by the time anyone calls this method - or else there's been a catastrophic failure.

Aaronontheweb · 2025-01-20T22:31:22Z

src/core/Akka/Actor/ActorCell.cs

@@ -216,7 +236,7 @@ public void Init(bool sendSupervise, MailboxType mailboxType)
            }

            SwapMailbox(mailbox);
-            Mailbox.SetActor(this);
+            Mailbox!.SetActor(this);


Mailbox can't be null by the time anyone calls this method - or else there's been a catastrophic failure.

Aaronontheweb · 2025-01-20T22:31:48Z

src/core/Akka/Actor/ActorCell.cs

+
+
+            Assert.Assert(instance != null, (string)(nameof(instance) + " != null"));
+            return instance!;


The assertion on the line above prevents this from ever actually returning null.

Aaronontheweb · 2025-01-20T22:32:55Z

src/core/Akka/Actor/ActorCell.cs

        public static IActorRef GetCurrentSenderOrNoSender()
        {
            var current = Current;
-            return current != null ? current.Sender : ActorRefs.NoSender;
+            return current?.Sender ?? ActorRefs.NoSender;


Since Current can be null and Sender can separately be null (this has always been a possibility), we return ActorRefs.NoSender here - which, ironically, is null right now.

Aaronontheweb · 2025-01-20T22:33:18Z

src/core/Akka/Actor/ChildrenContainer/Internal/ChildrenContainerBase.cs

@@ -188,19 +187,18 @@ public bool Contains(IActorRef actor)
        /// <param name="sb">TBD</param>
        /// <param name="kvp">TBD</param>
        /// <param name="index">TBD</param>
-        protected void ChildStatsAppender(StringBuilder sb, KeyValuePair<string, IChildStats> kvp, int index)
+        protected static void ChildStatsAppender(StringBuilder sb, KeyValuePair<string, IChildStats> kvp, int index)


Not a breaking change - moving to static would only change inheritors, not callers.

(and there could be no inheritors of this method before.)

* Add `NotNullWhen` compiler annotation attribute to all of the `bool TryX(out Y)` patterned functions * Add assertions on all shebangs to make sure we document why the nullability check was overriden

Aaronontheweb · 2025-01-21T17:28:32Z

src/core/Akka/Util/Internal/Extensions.cs

@@ -177,13 +177,15 @@ public static TimeSpan Min(this TimeSpan @this, TimeSpan other)
        /// <param name="enumerable">TBD</param>
        /// <param name="item">TBD</param>
        /// <returns>TBD</returns>
-        public static IEnumerable<T> Concat<T>(this IEnumerable<T> enumerable, T item)
+        #nullable enable
+        public static IEnumerable<T> Concat<T>(this IEnumerable<T>? enumerable, T item)


Aaronontheweb · 2025-01-21T17:30:36Z

src/core/Akka/Actor/ActorRef.cs

@@ -226,7 +226,9 @@ public static class ActorRefs
        /// Use this value as an argument to <see cref="ICanTell.Tell"/> if there is not actor to
        /// reply to (e.g. when sending from non-actor code).
        /// </summary>
-        public static readonly IActorRef NoSender = null;
+        #nullable enable
+        public static readonly IActorRef? NoSender = null;


Arkatufus

LGTM

Aaronontheweb added 5 commits January 20, 2025 14:46

cleaning up ActorCell and enabling nullability

d5a5388

working on nullability for ActorCell

ddb93c5

more nullability support

7fb6b6c

finished with nullability warnings

dabdca7

added API approvals

2fde95f

Aaronontheweb added akka-actor api-change labels Jan 20, 2025

Aaronontheweb added this to the 1.5.36 milestone Jan 20, 2025

added real XML-DOC comment

0e0d389

Aaronontheweb commented Jan 20, 2025

View reviewed changes

Improve nullability implementation

fd0c1c1

* Add `NotNullWhen` compiler annotation attribute to all of the `bool TryX(out Y)` patterned functions * Add assertions on all shebangs to make sure we document why the nullability check was overriden

Aaronontheweb commented Jan 21, 2025

View reviewed changes

Update API Approval list

ecad788

Arkatufus approved these changes Jan 21, 2025

View reviewed changes

Aaronontheweb merged commit 2ee5176 into akkadotnet:dev Jan 21, 2025
9 of 12 checks passed

Aaronontheweb deleted the optional-actor-telemetry branch January 21, 2025 18:53

Arkatufus mentioned this pull request Jan 22, 2025

Update RELEASE_NOTES.md for 1.5.36 release #7479

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

made `ActorCell` use `nullable` #7475

made `ActorCell` use `nullable` #7475

Aaronontheweb commented Jan 20, 2025 •

edited

Loading

Aaronontheweb commented Jan 20, 2025

Aaronontheweb left a comment

Aaronontheweb Jan 20, 2025

Aaronontheweb Jan 20, 2025

Aaronontheweb Jan 20, 2025

Aaronontheweb Jan 20, 2025

Aaronontheweb Jan 20, 2025

Aaronontheweb Jan 20, 2025

Aaronontheweb Jan 20, 2025

Aaronontheweb Jan 20, 2025

Aaronontheweb Jan 20, 2025

Aaronontheweb Jan 20, 2025

Aaronontheweb Jan 20, 2025

Aaronontheweb Jan 21, 2025

Aaronontheweb Jan 21, 2025

Arkatufus left a comment



		Assert.Assert(instance != null, (string)(nameof(instance) + " != null"));
		return instance!;

made ActorCell use nullable #7475

made ActorCell use nullable #7475

Conversation

Aaronontheweb commented Jan 20, 2025 • edited Loading

Changes

Checklist

Aaronontheweb commented Jan 20, 2025

Aaronontheweb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Arkatufus left a comment

Choose a reason for hiding this comment

made `ActorCell` use `nullable` #7475

made `ActorCell` use `nullable` #7475

Aaronontheweb commented Jan 20, 2025 •

edited

Loading