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 race condition during actor startup sequence #160

Merged
merged 5 commits into from
Aug 18, 2024

Conversation

troygilman0
Copy link
Contributor

This PR is intended to fix the issue #159.

The race condition occurs when a msg is sent to an actor while it is still processing the actor.Initialized or actor.Started msg. This can easily happen when loading some state from a db when processing one of these startup msgs.

I am proposing that the inbox only be started after the actor has processed all the startup msgs. This requires some changes to the inbox:

  • initialize the procStatus to "stopped"
  • when Start() is called, transition to "starting" and then "idle" status, and then call schedule() to process queued msgs

I have added a test case to test the proper ordering of msgs under this scenario.

@troygilman0
Copy link
Contributor Author

@anthdm

@anthdm
Copy link
Owner

anthdm commented Jun 23, 2024

@troygilman0 Thanks for this! Very good work.

actor/process_test.go Outdated Show resolved Hide resolved
@troygilman0 troygilman0 requested a review from anthdm June 24, 2024 19:25
@troygilman0
Copy link
Contributor Author

@anthdm please review

Copy link
Owner

@anthdm anthdm left a comment

Choose a reason for hiding this comment

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

Well done! Thanks a lot for this.

@anthdm
Copy link
Owner

anthdm commented Aug 18, 2024

@tprifti Can your add your review as well?

@anthdm anthdm merged commit 0921477 into anthdm:master Aug 18, 2024
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