Cancel Build should kill all the builder children processes

[3v1n0]

When triggering a build Sublime keep tracks only of the main process that it has launched, so when trying to cancel a build, the sub-processes that the build system might have launched keep to stay alive (at least, it happens here using make -jX and cmake)

This makes the "Cancel Build" action useless, at least in Linux or (I guess, looking at the code) Mac.

The problem is in the default implementation of exec.AsyncProcess, that only kills the parent process, but not all its children.

The fix is easy, and just using something like that fixes the issue:

--- a/Default/exec.py
+++ b/Default/exec.py
@@ -76,6 +76,7 @@
                 stdin=subprocess.PIPE,
                 startupinfo=startupinfo,
                 env=proc_env,
+                preexec_fn=os.setsid,
                 shell=False)
         elif shell_cmd and sys.platform == "linux":
             # Explicitly use /bin/bash on Linux, to keep Linux and OSX as
@@ -88,6 +89,7 @@
                 stdin=subprocess.PIPE,
                 startupinfo=startupinfo,
                 env=proc_env,
+                preexec_fn=os.setsid,
                 shell=False)
         else:
             # Old style build system, just do what it asks
@@ -98,6 +100,7 @@
                 stdin=subprocess.PIPE,
                 startupinfo=startupinfo,
                 env=proc_env,
+                preexec_fn=os.setsid,
                 shell=shell)
 
         if path:
@@ -111,7 +114,6 @@
 
     def kill(self):
         if not self.killed:
-            self.killed = True
             if sys.platform == "win32":
                 # terminate would not kill process opened by the shell cmd.exe,
                 # it will only kill cmd.exe leaving the child running
@@ -121,7 +123,9 @@
                     "taskkill /PID " + str(self.proc.pid),
                     startupinfo=startupinfo)
             else:
+                os.killpg(self.proc.pid, signal.SIGTERM)
                 self.proc.terminate()
+            self.killed = True
             self.listener = None
 
     def poll(self):

A temporary solution for users, is to just create a new plugin that replaces the default AsyncProcess implementation we care about, using something like:

import os, sys
import threading
import signal
import subprocess
import time

try:
    default_exec = __import__("Default").exec
except:
    default_exec = importlib.import_module("Default.exec")

try:
    default_AsyncProcess
except NameError:
    default_AsyncProcess = default_exec.AsyncProcess

# Encapsulates subprocess.Popen, forwarding stdout to a supplied
# ProcessListener (on a separate thread)
class AsyncProcess(default_AsyncProcess):
    def __init__(self, cmd, shell_cmd, env, listener, path="", shell=False):
        """ "path" and "shell" are options in build systems """

        if not shell_cmd and not cmd:
            raise ValueError("shell_cmd or cmd is required")

        if shell_cmd and not isinstance(shell_cmd, str):
            raise ValueError("shell_cmd must be a string")

        self.listener = listener
        self.killed = False

        self.start_time = time.time()

        # Hide the console window on Windows
        startupinfo = None
        if os.name == "nt":
            startupinfo = subprocess.STARTUPINFO()
            startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW

        # Set temporary PATH to locate executable in cmd
        if path:
            old_path = os.environ["PATH"]
            # The user decides in the build system whether he wants to append $PATH
            # or tuck it at the front: "$PATH;C:\\new\\path", "C:\\new\\path;$PATH"
            os.environ["PATH"] = os.path.expandvars(path)

        proc_env = os.environ.copy()
        proc_env.update(env)
        for k, v in proc_env.items():
            proc_env[k] = os.path.expandvars(v)

        if shell_cmd and sys.platform == "win32":
            # Use shell=True on Windows, so shell_cmd is passed through with the correct escaping
            self.proc = subprocess.Popen(
                shell_cmd,
                stdout=subprocess.PIPE,
                stderr=subprocess.PIPE,
                stdin=subprocess.PIPE,
                startupinfo=startupinfo,
                env=proc_env,
                shell=True)
        elif shell_cmd and sys.platform == "darwin":
            # Use a login shell on OSX, otherwise the users expected env vars won't be setup
            self.proc = subprocess.Popen(
                ["/bin/bash", "-l", "-c", shell_cmd],
                stdout=subprocess.PIPE,
                stderr=subprocess.PIPE,
                stdin=subprocess.PIPE,
                startupinfo=startupinfo,
                env=proc_env,
                preexec_fn=os.setsid,
                shell=False)
        elif shell_cmd and sys.platform == "linux":
            # Explicitly use /bin/bash on Linux, to keep Linux and OSX as
            # similar as possible. A login shell is explicitly not used for
            # linux, as it's not required
            self.proc = subprocess.Popen(
                ["/bin/bash", "-c", shell_cmd],
                stdout=subprocess.PIPE,
                stderr=subprocess.PIPE,
                stdin=subprocess.PIPE,
                startupinfo=startupinfo,
                env=proc_env,
                preexec_fn=os.setsid,
                shell=False)
        else:
            # Old style build system, just do what it asks
            self.proc = subprocess.Popen(
                cmd,
                stdout=subprocess.PIPE,
                stderr=subprocess.PIPE,
                stdin=subprocess.PIPE,
                startupinfo=startupinfo,
                env=proc_env,
                preexec_fn=os.setsid,
                shell=shell)

        if path:
            os.environ["PATH"] = old_path

        if self.proc.stdout:
            threading.Thread(target=self.read_stdout).start()

        if self.proc.stderr:
            threading.Thread(target=self.read_stderr).start()

    def kill(self):
        if not self.killed:
            if sys.platform == "win32":
                # terminate would not kill process opened by the shell cmd.exe,
                # it will only kill cmd.exe leaving the child running
                startupinfo = subprocess.STARTUPINFO()
                startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW
                subprocess.Popen(
                    "taskkill /PID " + str(self.proc.pid),
                    startupinfo=startupinfo)
            else:
                os.killpg(self.proc.pid, signal.SIGTERM)
                self.proc.terminate()
            self.killed = True
            self.listener = None


default_exec.AsyncProcess = AsyncProcess

[dogancelik]

This could be provided as an option and maybe make this default 👍

[ghost]

This is very helpful, I think we should make it default.
Without this, c++ program can't be terminated in Mac OS when we do 'cancel build'.

[3v1n0]

Diff and plugins valid up to version 3117 are (backup from first post):

--- a/Default/exec.py
+++ b/Default/exec.py
@@ -58,17 +58,19 @@
             # Use a login shell on OSX, otherwise the users expected env vars won't be setup
             self.proc = subprocess.Popen(["/bin/bash", "-l", "-c", shell_cmd], stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE, startupinfo=startupinfo, env=proc_env, shell=False,
-                )
+                preexec_fn=os.setsid)
         elif shell_cmd and sys.platform == "linux":
             # Explicitly use /bin/bash on Linux, to keep Linux and OSX as
             # similar as possible. A login shell is explicitly not used for
             # linux, as it's not required
             self.proc = subprocess.Popen(["/bin/bash", "-c", shell_cmd], stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE, startupinfo=startupinfo, env=proc_env, shell=False)
+                stderr=subprocess.PIPE, startupinfo=startupinfo, env=proc_env, shell=False,
+                preexec_fn=os.setsid)
         else:
             # Old style build system, just do what it asks
             self.proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE, startupinfo=startupinfo, env=proc_env, shell=shell)
+                stderr=subprocess.PIPE, startupinfo=startupinfo, env=proc_env,
+                shell=shell, preexec_fn=os.setsid)

         if path:
             os.environ["PATH"] = old_path
@@ -81,7 +83,6 @@

     def kill(self):
         if not self.killed:
-            self.killed = True
             if sys.platform == "win32":
                 # terminate would not kill process opened by the shell cmd.exe, it will only kill
                 # cmd.exe leaving the child running
@@ -89,7 +90,9 @@
                 startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW
                 subprocess.Popen("taskkill /PID " + str(self.proc.pid), startupinfo=startupinfo)
             else:
+                os.killpg(self.proc.pid, signal.SIGTERM)
                 self.proc.terminate()
+            self.killed = True
             self.listener = None

     def poll(self):

--

import os, sys
import threading
import signal
import subprocess
import time

try:
    default_exec = __import__("Default").exec
except:
    default_exec = importlib.import_module("Default.exec")

try:
    default_AsyncProcess
except NameError:
    default_AsyncProcess = default_exec.AsyncProcess

# Encapsulates subprocess.Popen, forwarding stdout to a supplied
# ProcessListener (on a separate thread)
class AsyncProcess(default_AsyncProcess):
    def __init__(self, cmd, shell_cmd, env, listener,
            # "path" is an option in build systems
            path="",
            # "shell" is an options in build systems
            shell=False):

        if not shell_cmd and not cmd:
            raise ValueError("shell_cmd or cmd is required")

        if shell_cmd and not isinstance(shell_cmd, str):
            raise ValueError("shell_cmd must be a string")

        self.listener = listener
        self.killed = False

        self.start_time = time.time()

        # Hide the console window on Windows
        startupinfo = None
        if os.name == "nt":
            startupinfo = subprocess.STARTUPINFO()
            startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW

        # Set temporary PATH to locate executable in cmd
        if path:
            old_path = os.environ["PATH"]
            # The user decides in the build system whether he wants to append $PATH
            # or tuck it at the front: "$PATH;C:\\new\\path", "C:\\new\\path;$PATH"
            os.environ["PATH"] = os.path.expandvars(path)

        proc_env = os.environ.copy()
        proc_env.update(env)
        for k, v in proc_env.items():
            proc_env[k] = os.path.expandvars(v)

        if shell_cmd and sys.platform == "win32":
            # Use shell=True on Windows, so shell_cmd is passed through with the correct escaping
            self.proc = subprocess.Popen(shell_cmd, stdout=subprocess.PIPE,
                stderr=subprocess.PIPE, startupinfo=startupinfo, env=proc_env, shell=True)
        elif shell_cmd and sys.platform == "darwin":
            # Use a login shell on OSX, otherwise the users expected env vars won't be setup
            self.proc = subprocess.Popen(["/bin/bash", "-l", "-c", shell_cmd], stdout=subprocess.PIPE,
                stderr=subprocess.PIPE, startupinfo=startupinfo, env=proc_env, shell=False,
                preexec_fn=os.setsid)
        elif shell_cmd and sys.platform == "linux":
            # Explicitly use /bin/bash on Linux, to keep Linux and OSX as
            # similar as possible. A login shell is explicitly not used for
            # linux, as it's not required
            self.proc = subprocess.Popen(["/bin/bash", "-c", shell_cmd], stdout=subprocess.PIPE,
                stderr=subprocess.PIPE, startupinfo=startupinfo, env=proc_env, shell=False,
                preexec_fn=os.setsid)
        else:
            # Old style build system, just do what it asks
            self.proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
                stderr=subprocess.PIPE, startupinfo=startupinfo, env=proc_env,
                shell=shell, preexec_fn=os.setsid)

        if path:
            os.environ["PATH"] = old_path

        if self.proc.stdout:
            threading.Thread(target=self.read_stdout).start()

        if self.proc.stderr:
            threading.Thread(target=self.read_stderr).start()

    def kill(self):
        if not self.killed:
            if sys.platform == "win32":
                # terminate would not kill process opened by the shell cmd.exe, it will only kill
                # cmd.exe leaving the child running
                startupinfo = subprocess.STARTUPINFO()
                startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW
                subprocess.Popen("taskkill /PID " + str(self.proc.pid), startupinfo=startupinfo)
            else:
                os.killpg(self.proc.pid, signal.SIGTERM)
                self.proc.terminate()
            self.killed = True
            self.listener = None


default_exec.AsyncProcess = AsyncProcess

[FichteFoll]

What about Windows? Does taskkill already do this properly?

[3v1n0]

Mh, I'm not sure if passing that will work there too... I'm not familiar with windows at all, so someone should try that.

[FichteFoll]

I just checked. Adding the /F and /T parameters should do. /T kills a process and its childs and /F forces the close. I don't know what exactly can prevent processes, but my py test process refused to terminate because it still had a "running subprocess".
See taskkill /? for help (my system language is german so I won't paste my output here).

Thus, we get subprocess.Popen("taskkill /PID %d /T /F" % self.proc.pid, startupinfo=startupinfo).

[abbr]

@FichteFoll , any updates on this issue? I am using Windows and I can reproduce the problem. Adding /F /T indeed fixed it. It would be great if the fix can be included in next release.

[wbond]

This was fixed in build 3158 and 3159.