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
Akka.Actor: make Context.Watch
and Context.Unwatch
enforce #nullable
#7155
Akka.Actor: make Context.Watch
and Context.Unwatch
enforce #nullable
#7155
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments on nullability changes inside ActorCell.DeathWatch
.
@@ -4,13 +4,12 @@ | |||
// Copyright (C) 2013-2023 .NET Foundation <https://github.com/akkadotnet/akka.net> | |||
// </copyright> | |||
//----------------------------------------------------------------------- | |||
|
|||
#nullable enable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Been seeing some NRE's coming from the Unwatch
handlers inside this class as part of the ActorLeakSpec
- decided to enable nullable
here to see what all is going on.
@@ -25,6 +24,7 @@ partial class ActorCell | |||
/// <returns>TBD</returns> | |||
public IActorRef Watch(IActorRef subject) | |||
{ | |||
if(subject is null) throw new ArgumentNullException(nameof(subject), "subject must not be null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is illegal. Should throw.
@@ -46,6 +46,7 @@ public IActorRef Watch(IActorRef subject) | |||
/// <returns>TBD</returns> | |||
public IActorRef WatchWith(IActorRef subject, object message) | |||
{ | |||
if(subject is null) throw new ArgumentNullException(nameof(subject), "subject must not be null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
@@ -69,6 +70,8 @@ public IActorRef WatchWith(IActorRef subject, object message) | |||
/// <returns>TBD</returns> | |||
public IActorRef Unwatch(IActorRef subject) | |||
{ | |||
if(subject is null) throw new ArgumentNullException(nameof(subject), "subject must not be null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
|
||
// Bugfix: need to filter out the AssociationTermination messages for remote@127.0.0.1:2553 from the quarantine | ||
// case, otherwise those logs might get picked up during the next text case | ||
Sys.EventStream.Publish(new Mute(new WarningFilter( new ContainsString("Association with remote system akka.trttl.tcp://remote@127.0.0.1:2553 has failed")))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh, we're going to have to do this after every test case - Akka.Remote's retry / reconnect bits are what's aggravating this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ActorsLeakSpec
Context.Watch
and Context.Unwatch
enforce #nullable
Changes
One of the raciest tests in the suite - looks like there's some residual slopiness with how we use the
EventFilter
plus one potential race condition withContext.Unwatch
that need to be fixed.Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):