Clean-up child processes on dispose

[renkun-ken]

What problem did you solve?

Closes #902
Closes #911

It looks like the behavior of child processes created by cp.spawn() and cp.exec() are platform-dependent in terms of whether they could be killed by tree-kill or cp.kill() in dispose().

This PR tracks all long-running child processes (e.g. help server, rmarkdown preview, langauge server) and explictly kill them on dispose so that it is unlikely to leave orphan processes when vscode exits.

Note that the way to kill process is platform-dependent. This PR uses cp.exec() in Windows and cp.spawn otherwise to start child process and uses tree-kill on Windows and cp.kill() on other platforms. It is the only way I find working on Windows, Linux, and macOS.

(If you do not have screenshot) How can I check this pull request?

1. Open an R file
2. Open a task manager or watch -d 'ps aux | grep /exec/R' to monitor R processes
3. Click "Clear cache and restart help server"
4. Open a Rmd file and preview.
5. Reload vscode window
6. Exit vscode

The help server, language server, and rmarkdown preview processes should be properly killed on reload or exit.

[renkun-ken]

I test this with Linux and macOS, and it works as expected. When I test it on a Windows virtual machine, the help server cannot be properly started. @ManuelHentschel Would you like to take a look on Windows?

[renkun-ken]

Now the help server could start on Windows but it never exits. When I quit vscode, those child processes do not exit, including the language server and its background workers. Looks like all the spawned processes do not automatically exit even when vscode exits. This does not happen on Linux and macOS.

[ElianHugh]

Is it possible that the shell argument to execSync is defaulting to true on Windows? Similarly, spawn has arguments that differ depending on platform, such as detached

For reference:

• https://nodejs.org/api/child_process.html#child_processexecsynccommand-options
• https://nodejs.org/api/child_process.html#child_processspawncommand-args-options

[renkun-ken]

Looks like specifying detached: false does not make it clean-up the child processes on exit on Windows automatically. Maybe we need to explicitly terminating all child processes (help server, language server) on exit on Windows.

[ElianHugh]

What if we make all processes disposable via util.asDisposable? These are pushed to the subscriptions array, so should be disposed of on exit regardless of platform

[renkun-ken]

Currently, we already push help server to the subscriptions where tree-kill is called in dispose(). However, it looks like only tree-kill could actually kill the process on Windows and only cp.kill() could kill it on Linux and macOS.

[renkun-ken]

We might need to keep track of all long-running child processes we create (e.g. help server, language server). dispose() of each child process should kill it only when it is still running, in case killing the wrong process via reused pid.

[renkun-ken]

Also, tree-kill still does not kill all the child processes of help server. The cmd.exe that starts R.exe, which starts RTerm.exe are not killed at all. In Windows, even when I quit vscode, there are still 17 child processes remaining:

[renkun-ken]

It looks like the following works:

• On Windows, cp.exec + tree-kill works.
• On Linux and macOS, cp.spawn + cp.kill() works.

We need to apply these to help server and language server so that all processes are cleaned up properly when vscode exits.

[ManuelHentschel]

I don't find a decent way to clean up child processes on Windows. @ManuelHentschel any ideas?

Sorry, I was little busy last week. I'll take closer look this weekend and try to come up with something!

[renkun-ken]

With the latest commit here, it should work on Windows, Linux and macOS now. I tested with all three platforms and you might want to confirm.

[renkun-ken]

I don't find a decent way to clean up child processes on Windows. @ManuelHentschel any ideas?

Sorry that was a mistake fixed via 31db66a. Should be working now.

[renkun-ken]

Through some testing, the latest commit should work with Windows and macOS but not Linux. Let me take another look on the Linux case where cp.exec() + cp.kill() does not work.

[renkun-ken]

On Linux, cp.spawn() + cp.kill() works.

[renkun-ken]

Through some testing, the latest commit works with Windows, Linux and macOS.

[ManuelHentschel]

I tested on windows 10 and ubuntu inside wsl, both worked fine. Thanks!

[beansrowning]

@renkun-ken I don't know if this is the same issue, but per my read, this is still broken on Windows 10 as of 2.3.5:

Over the course of the day working on several projects, this becomes quite unwieldy. Do I need to open a separate issue?

[renkun-ken]

@beansrowning Are they new processes or old orphan processes from the previous versions?

[leo-liar]

I still have this problem with the most recent version 2.3.5 on Win10, too ...

These processes are created every time I open .r-files in VSCode and all of them are orphaned after closing either the files or the whole editor.

[beansrowning]

I started from a fresh install of VSCode, installed VSCode-R via extensions (version 2.3.5), opened up an R project, and then closed the window and there are the 19 orphaned processes.

[renkun-ken]

Quote strange. @ManuelHentschel Do you think it might be related to certain Windows setup? For example, which terminal is used?

[Yunuuuu]

Maybe this can be helpful, which use following code to do this including both linux and windows:

import {exec} from 'child_process'

/**
 * kills the process and all its children
 * If you are on linux process needs to be launched in detached state
 * @param pid process identifier
 * @param signal kill signal
 */
export function killAll(pid:number, signal:string|number='SIGTERM'){
    if(process.platform == "win32"){
        exec(`taskkill /PID ${pid} /T /F`, (error, stdout, stderr)=>{
            console.log("taskkill stdout: " + stdout)
            console.log("taskkill stderr: " + stderr)
            if(error){
                console.log("error: " + error.message)
            }
        })
    }
    else{
        // see https://nodejs.org/api/child_process.html#child_process_options_detached
        // If pid is less than -1, then sig is sent to every process in the process group whose ID is -pid.
        process.kill(-pid, signal)
    }
}

the windows command taskkill is used to kill all child processes in windows.

[renkun-ken]

@Yunuuuu #936 uses taskkill to kill sub-processes in Windows. Would you like to try and see if it works correctly?

[renkun-ken]

I test it in a Windows environment, but it does not seem to work.

A lot of shell or wrapper processes do not exit.

[Yunuuuu]

Thanks for your great help! I have also tested it. The same with yours.

[Yunuuuu]

Just a workaround, I have implemented some R code to kill the excess r process . To use following code, we need install the ps package.

find_vscode <- function(pid) {

    ps_descented_handles <- ps::ps_descent(
        ps::ps_handle(pid)
    )

    ps_descented_handles[
        vapply(ps_descented_handles, function(h) {
            if (identical(ps::ps_name(h), "Code.exe")) TRUE else FALSE
        }, FUN.VALUE = logical(1L))
    ]

}

find_all_r_descentant_ps_handle <- function() {
    all_r_descentant_ps_handle <- lapply(ps::ps_pids(), function(pid) {
        tryCatch(
            { # nolint
                ps <- ps::ps_handle(pid)
                ps_name <- ps::ps_name(ps)
                if (identical(ps_name, "Rterm.exe")) {
                    ps::ps_descent(ps)
                } else {
                    NULL
                }
            },
            no_such_process = function(e) NULL,
            access_denied = function(e) NULL,
            error = function(e) NULL
        )
    })
    all_r_descentant_ps_handle[!vapply(
        all_r_descentant_ps_handle, is.null, FUN.VALUE = logical(1L)
    )]
}

#' keeped_r_pid       a integer atomic vector indicates the r pid keeped
kill_excess_r_process <- function(keeped_r_pid = Sys.getpid()) {
    keeped_vsc_ps_handle <- unlist(lapply(keeped_r_pid, find_vscode))
       keeped_vsc_pid <- vapply(keeped_vsc_ps_handle,
        ps::ps_pid,
        FUN.VALUE = integer(1L)
    )
    all_r_descentant_ps_handle <- find_all_r_descentant_ps_handle()

    r_ps_handle_pending_for_killing <- all_r_descentant_ps_handle[vapply(
        all_r_descentant_ps_handle, function(x) {
            pid <- vapply(x, ps::ps_pid, FUN.VALUE = integer(1L))
            if (any(pid %in% keeped_vsc_pid)) FALSE else TRUE
        },
        FUN.VALUE = logical(1L)
    )]

    if (identical(length(r_ps_handle_pending_for_killing), 0L)) {
        return(NULL)
    }

    r_ps_handle_pending_for_killing <- unlist(r_ps_handle_pending_for_killing)
    r_ps_handle_pending_for_killing <- r_ps_handle_pending_for_killing[vapply(
        r_ps_handle_pending_for_killing,
        function(ps_handle) {
            if (identical(ps::ps_name(ps_handle), "R.exe")) TRUE else FALSE
        },
        FUN.VALUE = logical(1L)
    )]

    r_pid_pending_for_killing <- vapply(
        r_ps_handle_pending_for_killing,
        ps::ps_pid,
        FUN.VALUE = integer(1L)
    )

    if (length(r_pid_pending_for_killing) > 0) {
        tryCatch(
            system2(
                "taskkill",
                args = c(
                    paste("/pid", r_pid_pending_for_killing),
                    "/t", "/f"
                ),
                stdout = FALSE, stderr = FALSE,
                wait = FALSE
            ),
            error = function(e) NULL
        )
    }
}

kill_excess_r_process(keeped_r_pid = Sys.getpid())

It seems taskkill can killed the corresponding process and all its child-process. I have tested it in R language. I don't know much about the conduct of vscode extension. @renkun-ken, maybe it's due to the PID of the process, which results in the ineffectiveness of the extension in windows?

[Yunuuuu]

To kill excess R.exe, without a running ancestor process "Code.exe", and their child processes automatically, add following code into our ~/Rprofile file.

    # Automatically kill excess R.exe without a running ancestor process "Code.exe"
    if (interactive() &&
        identical(Sys.getenv("TERM_PROGRAM"), "vscode")&&
        identical(Sys.info()[["sysname"]], "Windows")) {

        if (!"ps" %in% .packages(all.available = TRUE)) {
            message("Install package `ps` to kill excess `R.exe`!",
                appendLF = TRUE
            )
        } else {
            local({
                find_all_r_descentant_ps_handle <- function() {
                    all_r_descentant_ps_handle <- lapply(ps::ps_pids(), function(pid) {
                        tryCatch(
                            { # nolint
                                ps <- ps::ps_handle(pid)
                                ps_name <- ps::ps_name(ps)
                                if (identical(ps_name, "Rterm.exe")) {
                                    ps::ps_descent(ps)
                                } else {
                                    NULL
                                }
                            },
                            no_such_process = function(e) NULL,
                            access_denied = function(e) NULL,
                            error = function(e) NULL
                        )
                    })
                    all_r_descentant_ps_handle[!vapply(
                        all_r_descentant_ps_handle, is.null,
                        FUN.VALUE = logical(1L)
                    )]
                }

                kill_excess_r_process <- function() {
                    all_r_descentant_ps_handle <- find_all_r_descentant_ps_handle()

                    r_ps_handle_pending_for_killing <- all_r_descentant_ps_handle[vapply(
                        all_r_descentant_ps_handle, function(x) {
                            ps_name <- vapply(x, ps::ps_name, FUN.VALUE = character(1L))

                            if (!any(ps_name %in% "Code.exe")) TRUE else FALSE
                        },
                        FUN.VALUE = logical(1L)
                    )]

                    if (identical(length(r_ps_handle_pending_for_killing), 0L)) {
                        return(NULL)
                    }

                    r_ps_handle_pending_for_killing <- unlist(r_ps_handle_pending_for_killing)
                    r_ps_handle_pending_for_killing <- r_ps_handle_pending_for_killing[vapply(
                        r_ps_handle_pending_for_killing,
                        function(ps_handle) {
                            if (identical(ps::ps_name(ps_handle), "R.exe")) TRUE else FALSE
                        },
                        FUN.VALUE = logical(1L)
                    )]

                    r_pid_pending_for_killing <- vapply(
                        r_ps_handle_pending_for_killing,
                        ps::ps_pid,
                        FUN.VALUE = integer(1L)
                    )

                    if (length(r_pid_pending_for_killing) > 0) {
                        tryCatch(
                            system2(
                                "taskkill",
                                args = c(
                                    paste("/pid", r_pid_pending_for_killing),
                                    "/t", "/f"
                                ),
                                stdout = FALSE, stderr = FALSE,
                                wait = FALSE
                            ),
                            error = function(e) NULL
                        )
                    }
                }
                kill_excess_r_process()
            })
        }
    }

[Yunuuuu]

The ~/.Rprofile is running on the start of R.exe; by combining Autohotkey with above R script (keep code in local function), we can do it after vscode closed. The Autohotkey script can be as follows:

Loop
{
    WinWait, ahk_exe Code.exe
    WinWaitClose
    Sleep, 1000
    Run, the/path/of/Rscript.exe the/path/of/R_script_to_kill_excess_R.exe, , Hide
}

[renkun-ken]

@Yunuuuu Thanks for digging into the problem. Using an external script to clean up the processes might not be ideal in many cases. Please try the latest build from #936 and see if it works for you.

[Yunuuuu]

Thanks renkun-ken for great helping us in processing these problem. As discussed in #936, windows users can try to change your r.rpath.windows setting into your Rterm.exe path if the latest vscode-r version: 2.3.5 couldn't work for you to clean the excess child R process.