diff --git a/src/dvc_task/proc/manager.py b/src/dvc_task/proc/manager.py index e58ddaa..9909a71 100644 --- a/src/dvc_task/proc/manager.py +++ b/src/dvc_task/proc/manager.py @@ -111,7 +111,7 @@ def run_signature( immutable=immutable, ) - def send_signal(self, name: str, sig: int, group: bool = True): + def send_signal(self, name: str, sig: int, group: bool = False): """Send `signal` to the specified named process.""" try: process_info = self[name] @@ -135,9 +135,10 @@ def handle_closed_process(): if process_info.returncode is None: try: if sys.platform != "win32" and group: - os.killpg( # pylint: disable=no-member - process_info.pid, sig + pgid = os.getpgid( # pylint: disable=no-member + process_info.pid ) + os.killpg(pgid, sig) # pylint: disable=no-member else: os.kill(process_info.pid, sig) except ProcessLookupError: @@ -154,13 +155,18 @@ def handle_closed_process(): def interrupt(self, name: str, group: bool = True): """Send interrupt signal to specified named process""" - self.send_signal(name, signal.SIGINT, group) + if sys.platform == "win32": + self.send_signal( + name, signal.CTRL_C_EVENT, group # pylint: disable=no-member + ) + else: + self.send_signal(name, signal.SIGINT, group) - def terminate(self, name: str, group: bool = True): + def terminate(self, name: str, group: bool = False): """Terminate the specified named process.""" self.send_signal(name, signal.SIGTERM, group) - def kill(self, name: str, group: bool = True): + def kill(self, name: str, group: bool = False): """Kill the specified named process.""" if sys.platform == "win32": self.send_signal(name, signal.SIGTERM, group) diff --git a/tests/proc/test_manager.py b/tests/proc/test_manager.py index e60213c..9ab7021 100644 --- a/tests/proc/test_manager.py +++ b/tests/proc/test_manager.py @@ -29,13 +29,25 @@ def test_send_signal( mock_kill.assert_called_once_with(PID_RUNNING, signal.SIGTERM) if sys.platform != "win32": + gid = 100 + mocker.patch("os.getpgid", return_value=gid) mock_killpg = mocker.patch("os.killpg") process_manager.send_signal(running_process, signal.SIGINT, True) - mock_killpg.assert_called_once_with(PID_RUNNING, signal.SIGINT) + mock_killpg.assert_called_once_with(gid, signal.SIGINT) + else: + mock_kill.reset_mock() + process_manager.send_signal( + running_process, + signal.CTRL_C_EVENT, # pylint: disable=no-member + True, + ) + mock_kill.assert_called_once_with( + PID_RUNNING, signal.CTRL_C_EVENT # pylint: disable=no-member + ) mock_kill.reset_mock() with pytest.raises(ProcessLookupError): - process_manager.send_signal(finished_process, signal.SIGTERM) + process_manager.send_signal(finished_process, signal.SIGTERM, False) mock_kill.assert_not_called() if sys.platform == "win32": @@ -59,25 +71,27 @@ def side_effect(*args): mocker.patch("os.kill", side_effect=side_effect) with pytest.raises(ProcessLookupError): - process_manager.send_signal(running_process, signal.SIGTERM) + process_manager.send_signal(running_process, signal.SIGTERM, False) assert process_manager[running_process].returncode == -1 with pytest.raises(ProcessLookupError): - process_manager.send_signal("nonexists", signal.SIGTERM) + process_manager.send_signal("nonexists", signal.SIGTERM, False) if sys.platform == "win32": SIGKILL = signal.SIGTERM + SIGINT = signal.CTRL_C_EVENT # pylint: disable=no-member else: SIGKILL = signal.SIGKILL # pylint: disable=no-member + SIGINT = signal.SIGINT @pytest.mark.parametrize( - "method, sig", + "method, sig, group", [ - ("kill", SIGKILL), - ("terminate", signal.SIGTERM), - ("interrupt", signal.SIGINT), + ("kill", SIGKILL, False), + ("terminate", signal.SIGTERM, False), + ("interrupt", SIGINT, True), ], ) def test_kill_commands( @@ -85,13 +99,14 @@ def test_kill_commands( process_manager: ProcessManager, method: str, sig: signal.Signals, + group: bool, ): """Test shortcut for different signals.""" name = "process" mock_kill = mocker.patch.object(process_manager, "send_signal") func = getattr(process_manager, method) func(name) - mock_kill.assert_called_once_with(name, sig, True) + mock_kill.assert_called_once_with(name, sig, group) def test_remove(