System::stop_on_panic is removed. about actix-net HOT 10 OPEN

@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);
        }));

from actix-net.

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

from actix-net.

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:

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.

from actix-net.

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.

from actix-net.

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.

from actix-net.

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.

from actix-net.

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)

from actix-net.

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.

from actix-net.

@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.

from actix-net.

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.

from actix-net.