System::stop_on_panic is removed.

[mikhailOK]

I'm using actix-rt 1.0.0 through actix 0.9.0 and observing that system does not stop on panic. It seems like task panics get caught by tokio runtime 0.2.6.

Relevant tokio issue: https://github.com/tokio-rs/tokio/issues/2002

It also seems like actix-rt 0.2 always stopped on panic due to tokio 0.1 not catching.

My current workaround is to set a panic hook that sends a stop signal to current system, so another question: is it possible to make System::with_current do nothing if current system is not set? System::current() and System::with_current require the caller to guarantee it's set.

[XX]

@mikhailOK How do you emulate the stop on panic with panic hook?

[mikhailOK]

@XX currently this way:

        let default_hook = std::panic::take_hook();
        std::panic::set_hook(Box::new(move |info| {
            actix::System::with_current(|sys| sys.stop_with_code(1));
            default_hook(info);
        }));

[adwhit]

I dug into this a little bit. It seems that actix-server code still assumes that a worker can crash (i.e. uncaught panic) and it has a recovery mechanism in place; whereas with the newer tokio that is not really possible.

See for example here: https://github.com/actix/actix-net/blob/235a76dcd471dba6854ebe5929ff72c85c378f6e/actix-server/src/builder.rs#L410-L438 and here: https://github.com/actix/actix-net/blob/235a76dcd471dba6854ebe5929ff72c85c378f6e/actix-server/src/accept.rs#L419-L431

tokio do not seem to want to change the current behaviour (mandatory catching of panics within tokio) so we should consider a refactor to reflect that.

[fakeshadow]

use panic = "abort" if you want to exit on panic. tokio::spawn catch panic the same way as std::thread::spawn

actix-server can be used stand alone and you are free to make worker panic and restart. So worker restart feature is here to stay and remove it is not accepted.

[mikhailOK]

Seems like #255 removed the stop_on_panic method from System.

panic = "abort" is not always usable because it does not run destructors. std::thread::spawn returns a JoinHandle which allows the code to handle the panic. Please consider a feature to handle panic behavior.

[fakeshadow]

Sorry that's outside the scope of actix-server.

You can add your own panic handle by using std::panic module or use tokio::spawn which give you a joinhandle.

[mikhailOK]

Would panic handling functionality make sense for Actor then, considering Actor::start swallows the JoinHandle? (This is outside of actix-rt, but originally stop_on_panic was here)

[fakeshadow]

Would panic handling functionality make sense for Actor then, considering Actor::start swallows the JoinHandle? (This is outside of actix-rt, but originally stop_on_panic was here)

actix-rt and actix-server are different crates. And I was mostly explain how you can make actix-server worker panic and cause restart.

I did not make the change to stop_on_panic but since it's removed it would have not effect for sure. I can re-open the issue if you want it back or feel free to make new issue for talking about that.

[fakeshadow]

@mikhailOK I edited the title of issue and re-opened. Hope you don't mind as we need it back to implement actual panic stop.

[PhotonQuantum]

I walked into the exact problem, and when I ported the deleted snippet in #255 to actix 0.12.0 (here), it worked. Not sure why stop_on_panic is removed.

I also tested this idea on actix-web 4.0.0-beta.8. The program didn't terminate, but new service workers are constructed to accept new connections. I believe this is the expected behavior.