Ask should push unhandled answers into deadletter #5221
Conversation
|
I'll take a look! |
|
sry, for the micro commits Perf. increase for sealed classes This does not apply for IActorRef and ICanTell |
|
|
|
For the |
that takes me back to 2014, when I argued for using an |
|
Your view has been 100% on point, virt calls have a coast. But for IActorRef.Tel(msg) it has ... for sure. |
| { | ||
| if (Interlocked.Exchange(ref status, COMPLETED) == INITIATED) |
There was a problem hiding this comment.
Do we lose thread-safety without this? What happens if multiple threads try to complete the same Ask at the same time?
There was a problem hiding this comment.
TaskCompletionSource<T> _result is thread-safe, the barrier is not needed.
| @@ -121,43 +104,37 @@ public override IActorRefProvider Provider | |||
| /// <param name="sender">TBD</param> | |||
| protected override void TellInternal(object message, IActorRef sender) | |||
| { | |||
| var handled = false; | |||
There was a problem hiding this comment.
handled is a local variable, so if we attempt to send multiple messages to this FutureActorRef<T> this will evaluate to true each time, will it not?
There was a problem hiding this comment.
TaskCompletionSource<T> _result is thread-safe handled tracks if the current message could be set as the task result.
This reverts commit cac54cb.
There was a problem hiding this comment.
@Aaronontheweb What is missing and what question is unanswered?
| @@ -121,43 +104,37 @@ public override IActorRefProvider Provider | |||
| /// <param name="sender">TBD</param> | |||
| protected override void TellInternal(object message, IActorRef sender) | |||
| { | |||
| var handled = false; | |||
There was a problem hiding this comment.
TaskCompletionSource<T> _result is thread-safe handled tracks if the current message could be set as the task result.
| { | ||
| if (Interlocked.Exchange(ref status, COMPLETED) == INITIATED) |
There was a problem hiding this comment.
TaskCompletionSource<T> _result is thread-safe, the barrier is not needed.
| } | ||
| } | ||
| case ISystemMessage sysM: | ||
| SendSystemMessage(sysM); //we have special handling for system messages |
There was a problem hiding this comment.
I would throw an exception here - not supposed to send any system messages to temporary actors
|
@Zetanova thanks! That answers both of those questions - only one last issue and then I think we're good to go: https://github.com/akkadotnet/akka.net/pull/5221/files#r703000593 Would you mind submitting a new PR that implements that? Sorry for the screw-up here, again |
Currently invalid or timed out answers of Ask getting invisibly ignored by FutureActorRef
This PR will push all invalid answers into deadletter.
Invalid answers are timeout-answer, too many answers, answer in a bad state of FutureActorRef.
The only exception is a canceled Ask, this answer will still be ignored.