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

Actor doesn't fully stops when stopping returns Running::continue at least once #332

Open
Jonathas-Conceicao opened this issue Feb 11, 2020 · 2 comments

Comments

@Jonathas-Conceicao
Copy link
Member

I have been experimenting the current actix's Async Context implementation, Context, and I've come across a characteristic that I don't know if its a bug or the expected behavior. Check the following code:

// [dependencies]
// actix = "0.9"
// actix-rt = "1"
// tokio = { version = "0.2", features = ["rt-core"] }
use actix::prelude::*;
use std::sync::{Arc, Mutex};

struct Foo(Arc<Mutex<usize>>);

impl Actor for Foo {
    type Context = Context<Self>;

    fn stopping(&mut self, _: &mut Context<Self>) -> Running {
        let mut data = self.0.lock().unwrap();
        *data = 6;
        Running::Stop
    }
}

#[actix_rt::main]
async fn main() {
    let data = Arc::new(Mutex::new(0));
    let act = Foo(data.clone());
    act.start();
    loop {
        if dbg!(*data.lock().unwrap()) > 5 {
            break;
        }
        // Allows the Actor's future to be processed
        tokio::task::yield_now().await;
    }
}

This exemple naturally stops, since the Actor will have no live Address holders and no futures to process, when stopping the stopping method is called setting the shared data which unlocks the main function.

Now if we change the stopping method to the following:

    fn stopping(&mut self, _: &mut Context<Self>) -> Running {
        let mut data = self.0.lock().unwrap();
        if *data < 6 {
            *data += 1;
            return Running::Continue;
        }

        Running::Stop
    }

I would expect that the stopping method wold be called multiple times, and that eventually the Actor would fully stop. What happens in fact is that the Actor's future will be pending when it first returns Running::Continue, but then it won't ever be waken. Running this on gdb makes it easy to see that the Future stops being polled.

I have a simple solution for it which wold be adding a cx.waker().wake_by_ref(); to the Future when it becomes Pending due to Running::Continue returned by the stopping method. That way the Future wold immediately be available to be polled by the runtime again and this example would finish.

What do you guys think? Should the Actor future be waking itself in case of stopping returning a Running::Continue, or should it only be waken by external sources, e.g, new message being received.

@fakeshadow
Copy link
Contributor

fakeshadow commented Mar 9, 2021

I find this example can be considered anti pattern.
In an actor pattern the actor state should be mutated and accessed through messages.
A shared state that is mutated out side the context of actor is what causes problem here. There is no way to observe the change unless like you said add self wake up of actor.
The first iter works solely by accident. As actix rt is single thread and until your yield the actor context has not been poll so it can observe the first change.

Note: I'm not saying the problem is non exist but one that cause this in actor handler method would be more proper than the current one.

@ababo
Copy link

ababo commented Nov 12, 2022

I experience an opposite behaviour. Sometimes my websocket handling actors are dropped off after returning Running::Continue. Indeed I need to do some post-processing when connection is closing and some of these finalisation tasks are cancelled for unknown reason. This is reproduced intermittently. Really disappointing.

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

No branches or pull requests

3 participants