-
Notifications
You must be signed in to change notification settings - Fork 23
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
restore original procmask for child process #50
base: master
Are you sure you want to change the base?
Conversation
sigaction(SIGINT, &sig, NULL); | ||
sig.sa_handler = SIG_IGN; | ||
sigaction(SIGQUIT, &sig, NULL); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct? Ignore CTRL-C when running in the foreground?
I rather have #52 than this one merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This review of course applies to any PR with these two commits.
Perhaps merging this one first and then the other ones might be easier?
@@ -87,7 +87,7 @@ void DeleteConnection(connectionItem *ci); | |||
// constructors are public: | |||
|
|||
// processFactory creates the process that we are managing | |||
connectionItem * processFactory(char *exe, char *argv[]); | |||
connectionItem * processFactory(char *exe, char *argv[], sigset_t *sigset); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be const sigset_t *sigset
friend connectionItem * processFactory(char *exe, char *argv[], sigset_t *sigset); | ||
friend bool processFactoryNeedsRestart(); | ||
friend void processFactorySendSignal(int signal); | ||
public: | ||
processClass(char *exe, char *argv[]); | ||
processClass(char *exe, char *argv[], sigset_t *sigset); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for these
sigaddset(&sigset_block, SIGXFSZ); | ||
if (inFgMode) { | ||
sigaddset(&sigset_block, SIGINT); | ||
sigaddset(&sigset_block, SIGQUIT); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This unblocks some more signals from the child, because the SIG_IGN
disposition used in sigaction
previously would still apply to the child processes, even if the sigmask was restored.
Would be nice if the commit message called out that nuance.
Fix for issue #48.