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.

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

bug in SIGUSR2 handling for cancelling commands #4755

Open
elliefm opened this issue Nov 30, 2023 · 2 comments
Open

bug in SIGUSR2 handling for cancelling commands #4755

elliefm opened this issue Nov 30, 2023 · 2 comments

Comments

@elliefm
Copy link
Contributor

elliefm commented Nov 30, 2023

Looks like we have an undocumented feature by which an admin can cancel some long-running user commands, by sending SIGUSR2 to the process servicing that client -- see cmd_cancelled() in global.c and the various places that use it.

But, we don't quite install the SIGUSR2 signal handler correctly. In signals_add_handlers(), where we set up the signal handler, we have this a few lines above:

    action.sa_flags |= SA_RESETHAND;

This flag says that the handler should be reset to this signal's default after it has been handled once. For SIGUSR2, the default is to terminate the process. This means that, an administrator can cancel one user command per process lifetime. If they later try to cancel another user command in the same process, the process will just terminate (without even a clean shutdown). This seems surprising! And perhaps the only reason nobody gets surprised by it is that nobody knows this feature exists, because the only way to find it is to be reading the signals code...

SA_RESET_HAND makes sense for the other signals set up in the same breath (SIGQUIT, SIGINT, SIGTERM): for those cases, if the process hasn't responded to the request to exit cleanly in a timely manner, you want the next signal to just terminate it. But it makes no sense for SIGUSR2.

I'm not sure how useful this feature is, and maybe it's bitrotted anyway. We might decide to discard it rather than fix it...

@elliefm
Copy link
Contributor Author

elliefm commented Nov 30, 2023

Oh! This can also be activated by the user themselves, using the XKILLMY imap command... which is also not documented anywhere.

@elliefm
Copy link
Contributor Author

elliefm commented Nov 30, 2023

In that case maybe it's intentional that a second signal terminates the process

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant