Skip to content
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.

Sign up for GitHub

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

Jump to bottom

#152 Handle Ctrl+C for _run and user commands #162

Closed
wants to merge 2 commits into from
Closed

#152 Handle Ctrl+C for _run and user commands #162

wants to merge 2 commits into from

Conversation

anti-social
Copy link
Contributor

No description provided.

@anti-social
Copy link
Contributor Author

Also we should correctly handle Ctrl+Z, but there some troubles with it. Vagga wrapper can't send SIGSTOP to self. So wrapper should ask parent process that is outside of namespace to stop wrapper.

tailhook
tailhook reviewed Jan 26, 2016
View reviewed changes
src/tty_util.rs
}

pub fn ignore_tty_signals() -> Result<(), String> {
let tty_signals = vec![SIGTTIN, SIGTTOU];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not "rusty" to allocate a vector just to iterate over the items. Should just call sigaction() twice. (And yes sigaction is a safe wrapper around the same stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't use sigaction from nix package together with SIG_IGN: nix-rust/nix#241

@anti-social anti-social force-pushed the handle_sigint branch 2 times, most recently from c87bc92 to dd3f98f Compare February 3, 2016 12:26
@anti-social
Copy link
Contributor Author

Should use killpg to send SIGCONT signal. TODO: fix and add test case.

@tailhook tailhook mentioned this pull request Feb 12, 2016
Closed
tailhook
tailhook reviewed Feb 12, 2016
View reviewed changes
src/process_util.rs
}
SIGTERM => {
// SIGTERM is usually sent to a specific process so we
// forward it to children
debug!("Received SIGTERM signal, propagating");
child.signal(SIGTERM).ok();
try!(child.signal(SIGTERM)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you think it's a fatal error? The error was ignored because we can't ensure that process is still alive.

tailhook added a commit that referenced this pull request Mar 12, 2016
@tailhook
Improved SIGINT handling for interactive commands
fa38dce 
Fixes #152. Supercedes #162. Squashed from #168.

This also breaks handling of Ctrl+Z hopefully it's not a big problem for
most workaloads. As fixing it requires bigger effort.
@tailhook
Copy link
Owner

Merged as part of #168

@tailhook tailhook closed this Mar 12, 2016
@anti-social anti-social deleted the handle_sigint branch March 20, 2017 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@anti-social @tailhook