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

Fixes for NoSender serialization bugs #1037

Merged

Conversation

ssoshchepkov
Copy link
Contributor

Fixes for NoSender actor serialization bugs described in #1036.

@ssoshchepkov ssoshchepkov changed the title Fixes for NoSender serialization bugs #1036 Fixes for NoSender serialization bugs Jun 3, 2015
@Aaronontheweb
Copy link
Member

thanks, @ssoshchepkov - looks like we've overlooked this one.

Aaronontheweb added a commit that referenced this pull request Jun 3, 2015
…-1036

Fixes for NoSender serialization bugs
@Aaronontheweb Aaronontheweb merged commit cd7aee8 into akkadotnet:dev Jun 3, 2015
@rogeralsing
Copy link
Contributor

Im not sure this fix is correct.
The scala version of the code uses the old "/nosender" path.
I think there could be more to this than we see here

@Aaronontheweb
Copy link
Member

@rogeralsing ok, I reverted it. I'll let you take a closer look and see what's really going on.

@rogeralsing
Copy link
Contributor

Turns out that its NoBody that uses that notation in scala.
NoSender in scala is just a wrapper for null

@ssoshchepkov if you want, could you adapt the PR to treat NoSender as null instead?
It's better if we stay as close as possible to the Scala code.

  /**
   * Default placeholder (null) used for "!" to indicate that there is no sender of the message,
   * that will be translated to the receiving system's deadLetters.
   */
  final val noSender: ActorRef = null

@Aaronontheweb
Copy link
Member

@rogeralsing won't that conflict with your PR that turns null into NoSender? :p

@rogeralsing
Copy link
Contributor

That code could just go away if we do this.
There is no reason to have weird comparisons all over the codebase if nosender really means null imo.

Or?

@ssoshchepkov
Copy link
Contributor Author

@rogeralsing I've made changes to treat NoSender as null. Could you reopen this PR? Or should I create another?

@rogeralsing
Copy link
Contributor

I think you have to send a fresh one. I dont think a PR can be re-opened once merged (as this was merged and then a revert patch applied)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants