-
Notifications
You must be signed in to change notification settings - Fork 265
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
In multi-trigger, if one trigger crashes, others may keep running #2245
Comments
Confirmed finalisers do not appear to run. |
@itowlson I was able to repro this. With a #!/bin/bash
i=0
while true; do
echo $i
i=$(( i+1 ))
sleep .5
done Interestingly adding let mut command = Command::new(binary);
command.args(args);
command.envs(get_env_vars_map()?);
command.kill_on_drop(true); |
Oh interesting! How did you stop the Spin process? In my case the parent process is |
I had to add a Ctrl+C handler to |
I cannot repro my success with my changes, so I agree that |
AHHH THE CTRL+C HANDLER WORKED AND NOW IT DOESN'T CURSE YOU ALL |
(not you Kate. just the unfeeling cosmos in general) |
oh SOMETIMES it works |
I created a multi-trigger app with http, timer and sqs triggers. I knew that the SQS trigger was not yet compatible with multi-trigger. The SQS trigger obligingly crashed during startup, but the timer trigger continued to run.
I believe this is due to the timer trigger being a plugin. To run the trigger, we start a
spin
process, which in turn starts atimer-trigger
process. I believe that when we kill thatspin
process, the childtimer-trigger
process continues to run.Further evidence for this is from logging the PIDs of the trigger processes. None of these PIDs matched the ID from doing
ps -A | grep timer
after SQS had crashed.This may be a bit gnarly to fix because when we kill the Spin process it just dies, it doesn't get to say "goodnight sweet prince" or (more relevantly) "die, actual plugin process, die" - we can try setting
kill_on_drop
on the child but I am not sure if finalisers will run in this case. Anyway I will take a look at it and see if I can make a dent.cc @kate-goldenring if you have any thoughts on this!
The text was updated successfully, but these errors were encountered: