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: tokio-rs/tokio#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:

actix-net/actix-server/src/builder.rs

Lines 410 to 438 in 235a76d

	ServerCommand::WorkerFaulted(idx) => {
	let mut found = false;
	for i in 0..self.workers.len() {
	if self.workers[i].0 == idx {
	self.workers.swap_remove(i);
	found = true;
	break;
	}
	}
	if found {
	error!("Worker has died {:?}, restarting", idx);
	let mut new_idx = self.workers.len();
	'found: loop {
	for i in 0..self.workers.len() {
	if self.workers[i].0 == new_idx {
	new_idx += 1;
	continue 'found;
	}
	}
	break;
	}
	let worker = self.start_worker(new_idx, self.accept.get_notify());
	self.workers.push((new_idx, worker.clone()));
	self.accept.send(Command::Worker(worker));
	}
	}

and here:

actix-net/actix-server/src/accept.rs

Lines 419 to 431 in 235a76d

	Err(tmp) => {
	self.srv.worker_faulted(self.workers[self.next].idx);
	msg = tmp;
	self.workers.swap_remove(self.next);
	if self.workers.is_empty() {
	error!("No workers");
	self.backpressure(true);
	return;
	} else if self.workers.len() <= self.next {
	self.next = 0;
	}
	continue;
	}

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.