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

Rework handle_while #83

Merged
merged 10 commits into from
Jun 16, 2022
Merged

Rework handle_while #83

merged 10 commits into from
Jun 16, 2022

Conversation

Restioson
Copy link
Owner

This PR renames handle_while to join, fixing a potential issue that would cause slight inefficiency if the actor is stopped during handling. It also adds select as a counterpart, which is like join but will return early if the actor is stopped.

Restioson added a commit that referenced this pull request Jun 10, 2022
@Restioson Restioson added the enhancement New feature or request label Jun 14, 2022
@Restioson
Copy link
Owner Author

There's also a minor internal spelling error fix in here which can be extracted if needed.

@@ -341,12 +343,115 @@ impl<A: Actor> Context<A> {
let mut addr_recv = addr_rx.recv_async();
let mut broadcast_recv = broadcast_rx.recv_async();

loop {
while self.running == RunningState::Running {
let (next_msg, unfinished) = {
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is currently a lot of code duplication, but hopefully if #85 makes it in it should be able to be simplified quite a lot

@Restioson
Copy link
Owner Author

Restioson commented Jun 14, 2022

Some tests for join and select might be good, but those probably don't need to be reviewed for design, so this is ready for review, I think.

Copy link
Collaborator

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

Concept ACK

Borrowing existing ecosystem terminology here is definitely a plus.

@Restioson
Copy link
Owner Author

Ready for merge?

Copy link
Contributor

@klochowicz klochowicz left a comment

Choose a reason for hiding this comment

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

LGTM!

@Restioson Restioson merged commit d5f316e into master Jun 16, 2022
@Restioson Restioson added this to the 0.6.0 milestone Jun 16, 2022
Restioson added a commit that referenced this pull request Jun 17, 2022
* Remove stopping and repurpose KeepRunning

* Fix build - logic will be fixed in #83

* Change stop_all behaviour - in line with stop_self

Previously, stop_all would immediately disconnect the address. However, stop_self done on every actor would actually not do this in one case - if there were a free-floating (not executing an actor event loop) Context. This change brings stop_all in line with stop_self.

* Fmt + document more breaking changes

* merge oversight
@Restioson Restioson deleted the handle_while_rework branch June 24, 2022 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants