Skip to content

Update eval to use SIGTERM for killed processes#1480

Closed
Akarys42 wants to merge 2 commits into
mainfrom
snekbox-SIGTERM
Closed

Update eval to use SIGTERM for killed processes#1480
Akarys42 wants to merge 2 commits into
mainfrom
snekbox-SIGTERM

Conversation

@Akarys42
Copy link
Copy Markdown
Contributor

Due to a recent change in snekbox (python-discord/snekbox#96) processes killed by cgroup limits will use SIGTERM instead of SIGKILL. This commit account for this change by updating the numerical value to 15 (SIGTERM portable value).

Due to a recent change in snekbox (python-discord/snekbox#96) processes killed by cgroup limits will use SIGTERM instead of SIGKILL. This commit account for this change by updating the numerical value to 15 (SIGTERM portable value).
@Akarys42 Akarys42 added t: bug Something isn't working p: 1 - high High Priority a: utility Related to utility commands: (bot, eval, extensions, jams, reminders, snekbox, utils) labels Mar 22, 2021
@Akarys42 Akarys42 requested review from MarkKoz and jb3 March 22, 2021 13:52
@Akarys42 Akarys42 requested a review from ks129 as a code owner March 22, 2021 13:52
@MarkKoz
Copy link
Copy Markdown
Contributor

MarkKoz commented Mar 22, 2021

This is incorrect. SIGTERM is only used to stop the process when stdout is too large. Nsjail still uses SIGKILL for time, memory, and seccomp violations.

@Akarys42
Copy link
Copy Markdown
Contributor Author

This is incorrect. SIGTERM is only used to stop the process when stdout is too large. Nsjail still uses SIGKILL for time, memory, and seccomp violations.

Alright, I misunderstood the change then my bad. What about using SIGKILL and SIGTERM to trigger this message then?

@MarkKoz
Copy link
Copy Markdown
Contributor

MarkKoz commented Mar 22, 2021

If anything it should show an entirely separate message. SIGTERM will never have anything to do with the current message so it'd be inaccurate to use it.

@Akarys42
Copy link
Copy Markdown
Contributor Author

If anything it should show an entirely separate message. SIGTERM will never have anything to do with the current message so it'd be inaccurate to use it.

Alright, let’s do that in another PR then.

@Akarys42 Akarys42 closed this Mar 25, 2021
@Akarys42 Akarys42 deleted the snekbox-SIGTERM branch March 25, 2021 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: utility Related to utility commands: (bot, eval, extensions, jams, reminders, snekbox, utils) p: 1 - high High Priority t: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants