Merge ~kissiel/checkbox-ng:fix-non-killable-process-nests into checkbox-ng:master

Proposed by Maciej Kisielewski
Status: Merged
Approved by: Sylvain Pineau
Approved revision: f82f186bf68a95bde0b28efe9a8c556b2d0d4442
Merged at revision: 8df2f5a44fdc6cb06f3636dec018c479e912c2ec
Proposed branch: ~kissiel/checkbox-ng:fix-non-killable-process-nests
Merge into: checkbox-ng:master
Diff against target: 24 lines (+4/-2)
1 file modified
plainbox/impl/execution.py (+4/-2)
Reviewer Review Type Date Requested Status
Sylvain Pineau (community) Approve
Review via email: mp+372538@code.launchpad.net

Description of the change

This patch helps with interrupting runaway processes (groups of processes, to be precise).

For processes that had a structure like this:

foo | bar | baz

In case where foo command failed, if the operator hit ctrl+c the signal went to already non existent pid1, while pipes were still open to the whole chain.

This patch makes checkbox close the pipes when kill command is issued. It also changes the method of killing to the new one, which is more reliable when running root jobs.

Tested by running multiple weird jobs, including the nefarious `more file |cat`. With and without `user: root`.

To post a comment you must log in.
Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

+1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/plainbox/impl/execution.py b/plainbox/impl/execution.py
index 01e783f..db4d73e 100644
--- a/plainbox/impl/execution.py
+++ b/plainbox/impl/execution.py
@@ -216,6 +216,7 @@ class UnifiedRunner(IJobRunner):
216 time.sleep(0.1)216 time.sleep(0.1)
217 except BrokenPipeError:217 except BrokenPipeError:
218 pass218 pass
219 os.close(in_w)
219 forwarder_thread = threading.Thread(220 forwarder_thread = threading.Thread(
220 target=stdin_forwarder, args=(stdin,))221 target=stdin_forwarder, args=(stdin,))
221 forwarder_thread.start()222 forwarder_thread.start()
@@ -241,8 +242,9 @@ class UnifiedRunner(IJobRunner):
241 proc.wait()242 proc.wait()
242 break243 break
243 except KeyboardInterrupt:244 except KeyboardInterrupt:
244 # On interrupt send a signal to the process245 is_alive = False
245 extcmd_popen._on_keyboard_interrupt(proc)246 import signal
247 self.send_signal(signal.SIGKILL, target_user)
246 # And send a notification about this248 # And send a notification about this
247 extcmd_popen._delegate.on_interrupt()249 extcmd_popen._delegate.on_interrupt()
248 finally:250 finally:

Subscribers

People subscribed via source and target branches