Skip to content
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

fix NRE inside RemotingTerminator #4686

Merged

Conversation

Aaronontheweb
Copy link
Member

close #4677

This patch also fixes all NRE's originating from the logging system for any FSM<TState,TData inside Akka.NET

This patch also fixes all NRE's originating from the logging system for any FSM<TState,TData> inside Akka.NET
Applied some ReSharper-ing to cleanup the internal message processing of the FSM to take advantage of C#7/8 features
Copy link
Contributor

@IgorFedchenko IgorFedchenko left a comment

Choose a reason for hiding this comment

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

That's a great change - definitely need to make loggers creation to be a safe operation.

Left some comments, not sure about ones which are non-refactoring though

@@ -713,8 +713,7 @@ private void InitFSM()
{
When(TerminatorState.Uninitialized, @event =>
{
var internals = @event.FsmEvent as Internals;
if (internals != null)
if (@event.FsmEvent is Internals internals)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's always lovely to see how new language features make old code pretty 👍

src/core/Akka/Actor/FSM.cs Outdated Show resolved Hide resolved
src/core/Akka/Actor/FSM.cs Show resolved Hide resolved
Copy link
Contributor

@IgorFedchenko IgorFedchenko left a comment

Choose a reason for hiding this comment

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

LGTM

@Aaronontheweb Aaronontheweb merged commit 3f0d654 into akkadotnet:dev Dec 28, 2020
@Aaronontheweb Aaronontheweb deleted the fix/4677-NRE-remotingTerminator branch December 28, 2020 22:21
This was referenced Dec 30, 2020
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.

NullReferenceException when updating to 1.4.13 version
2 participants