Remove stopping and the entire KeepRunning construct

[thomaseizinger]

Whilst working on #51, I came to the conclusion that I'd like to propose to remove the stopping function and with it the concept of KeepRunning.

The reasons why I consider it problematic are:

• With KeepRunning::StopAll, we may terminate actors without them being able to prevent that. It seems weird that a single actor can intervene shutdown (when initiated via Context#stop) but other actors cannot.
• IMO, an actor should not be able to halt a shutdown when initiated via Context#stop. It seems much cleaner to use a supervisor pattern (see https://github.com/itchysats/itchysats/blob/master/xtras/src/supervisor.rs for example) to restart an actor when it was shut down. With the recent addition of Actor#Stop, this is actually super easy and clean to implement.

The current features of stopping and KeepRunning can still be achieved through other means:

• Shutting down all actors in a Context can be done with a custom message and the notify_all function
• Preventing an actor from shutdown is not possible but it can be restarted with a supervisor (see above)

Removing stopping and KeepRunning would IMO also simplify a lot of the state management in Context and make #51 easier to implement / finish. What do you think?

[Restioson]

This does make sense to me, I think. I really like how this simplifies control flow and general usage - less ways to do the same thing. You can emulate preventing an actor from shutdown from Actor::stopping by adding a ShutdownMaybe message which then checks if the actor should be shut down, and calls Context::stop if required.

[Restioson]

This would supercede and close #58, right?

[thomaseizinger]

This would supercede and close #58, right?

Yes.

[thomaseizinger]

This does make sense to me, I think. I really like how this simplifies control flow and general usage - less ways to do the same thing. You can emulate preventing an actor from shutdown from Actor::stopping by adding a ShutdownMaybe message which then checks if the actor should be shut down, and calls Context::stop if required.

Yep, I had a similar thought. I think moving more of this functionality into messages is a good thing.

I've also been contemplating the idea of making the started lifecycle hook a message but I am not fully sure about that yet :)

[Restioson]

How would replacing started work? Would each address of the actor be made to send a start message?

[thomaseizinger]

I was thinking of something like this (assuming #62):

impl Context {
	pub async fn run<A>(mut self, actor: A) -> A::Stop where A: Actor + Handler<Started, ()> {
		self.address.send(Started).await;
	}
}

But I just realized that that doesn't work because we obviously can't receive messages until we are in the actual loop within the context.

[thomaseizinger]

This does make sense to me, I think. I really like how this simplifies control flow and general usage - less ways to do the same thing. You can emulate preventing an actor from shutdown from Actor::stopping by adding a ShutdownMaybe message which then checks if the actor should be shut down, and calls Context::stop if required.

Cool. If you are on-board with that, I will have a go at removing the function and all the code associated with KeepRunning etc.

[Restioson]

Sure, and thanks! 👍

[Restioson]

Solved by #82 . Note that KeepRunning was kept for attach_stream but when this is moved to an extension crate it will go there too.