Skip to content

Conversation

@Arkatufus
Copy link
Contributor

Fixes #7794

Changes

Make sure ConcurrentAsyncCallback can handle null events properly.

Actual Bug

  • ChannelSource.OnReaderComplete accepts a nullable Exception as an argument (null sentinel) and was passed in as a logic async callback.
  • When this callback was invoked with null value, ConcurrentAsyncCallback would not accept null as a value and considered it as an invalid event and tried to raise an ArgumentException exception.
  • ConcurrentAsyncCallback throws a NullReferenceException because it tried to call obj.GetType() inside string extrapolation because obj is null.

Copy link
Contributor Author

@Arkatufus Arkatufus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review

handler1(e);
else
throw new ArgumentException(
$"Expected {nameof(obj)} to be of type {typeof(T)}, but was {obj.GetType()}");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The NRE was thrown here by obj.GetType() when obj is null.

The actual fix was to make sure that this wrapper also handles null and passes it to the handler delegate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, this should be fixed using #nullable enable later to make it explicit that T can be null

Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look tentatively good to me, so long as the test suite agrees

@Aaronontheweb Aaronontheweb merged commit 4168945 into akkadotnet:v1.5 Sep 4, 2025
9 of 11 checks passed
Arkatufus added a commit to Arkatufus/akka.net that referenced this pull request Sep 5, 2025
Aaronontheweb pushed a commit that referenced this pull request Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants