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 for async await suspend-resume mechanics #836

Merged
merged 1 commit into from
Apr 11, 2015
Merged

Fix for async await suspend-resume mechanics #836

merged 1 commit into from
Apr 11, 2015

Conversation

rogeralsing
Copy link
Contributor

Hotfix for async await suspend-resume mailbox mechanics.

@rogeralsing
Copy link
Contributor Author

cc @nvivo

There is really no excuse for this one, we rushed the v1 release.
The code failed whenever more than one message was sent to a async suspended actor.

We do have a test for this

public class SuspendActor : ReceiveActor
    {
        public SuspendActor()
        {
            var state = 0;
            Receive<string>(s => s == "change", _ =>
            {
                state = 1;
            });
            Receive<string>(AsyncBehavior.Suspend, async _ =>
            {
                Self.Tell("change");
                await Task.Delay(TimeSpan.FromSeconds(1));
                //we expect that state should not have changed due to an incoming message
                Sender.Tell(state);
            });
        }
    }

Can anyone spot the issue?
async methods in C# are really not async until they hit await so the Self.Tell("change") fired before the mailbox was suspended, thus making the test pass.

In general, I think we have been too sloppy on the test coverage side of things lately.

@nvivo
Copy link
Contributor

nvivo commented Apr 11, 2015

Thanks!

@Aaronontheweb
Copy link
Member

There is really no excuse for this one, we rushed the v1 release.There is really no excuse for this one, we rushed the v1 release.

We had several weeks to find this bug from the time the PR was accepted to when the NuGet package was pushed. The release was definitely not rushed. But bugs happen - not a big deal. Don't turn it into a fault-finding mission.

Aaronontheweb added a commit that referenced this pull request Apr 11, 2015
…otfix

Fix for async await suspend-resume mechanics
@Aaronontheweb Aaronontheweb merged commit 0cf29c9 into akkadotnet:dev Apr 11, 2015
This was referenced Apr 28, 2015
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.

None yet

3 participants