Windows7 で job_stop の kill が正しく動作しない #973

Closed
mattn opened this Issue Oct 20, 2016 · 22 comments

Projects

None yet

3 participants

@mattn
Member
mattn commented Oct 20, 2016

vim/vim#1184

#820

CreateToolhelp32Snapshot で出来るはず。

@mattn
Member
mattn commented Oct 20, 2016 edited
#include <windows.h>
#include <tlhelp32.h>

static void
terminate_all(HANDLE process, int code)
{
    PROCESSENTRY32 pe;
    HANDLE h = INVALID_HANDLE_VALUE;
    DWORD pid = GetProcessId(process);

    h = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
    if (h == INVALID_HANDLE_VALUE)
        return;

    pe.dwSize = sizeof(PROCESSENTRY32);
    if (!Process32First(h, &pe))
        return;

    while (1)
    {
        if (!Process32Next(h, &pe))
            break;
        if (pe.th32ParentProcessID == pid)
        {
            HANDLE ph = OpenProcess(
                PROCESS_ALL_ACCESS, FALSE, pe.th32ProcessID);
            if (ph != NULL)
            {
                terminate_all(ph, code);
                CloseHandle(ph);
            }
        }
    }
    CloseHandle(h);

    TerminateProcess(process, code);
}

こんな感じ?

@mattn
Member
mattn commented Oct 20, 2016

あ、まずそう。

@k-takata
Member

Process32Next はループの最後にすべきかと。最初のプロセスがスキップされてしまいそう。

@mattn
Member
mattn commented Oct 20, 2016

ですね。いま自分のプロセスが死んで気付いたところw

@mattn
Member
mattn commented Oct 20, 2016 edited
#include <windows.h>
#include <tlhelp32.h>
#include <stdio.h>

  static void
terminate_all(HANDLE process, int code)
{
    PROCESSENTRY32 pe;
    HANDLE h = INVALID_HANDLE_VALUE;
    DWORD pid = GetProcessId(process);

    h = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
    if (h == INVALID_HANDLE_VALUE)
        return;

    pe.dwSize = sizeof(PROCESSENTRY32);
    if (!Process32First(h, &pe))
        return;

    do
    {
        if (pe.th32ParentProcessID == pid)
        {
            HANDLE ph = OpenProcess(
                PROCESS_ALL_ACCESS, FALSE, pe.th32ProcessID);
            if (ph != NULL)
            {
                terminate_all(ph, code);
                CloseHandle(ph);
            }
        }
    } while (Process32Next(h, &pe));

    CloseHandle(h);
    TerminateProcess(process, code);
}

int
main(int argc, char* argv[]) {
    HANDLE ph = OpenProcess(
        PROCESS_ALL_ACCESS, FALSE, atoi(argv[1]));
    if (ph == NULL) exit(3);
    terminate_all(ph, 1);
}

よさげ。gvim 起動してそこから :sh して、msys2 で ps -W | grep gvim したプロセスIDを引数で渡したら cmd ごと死んだので行けてるはず。

@mattn
Member
mattn commented Oct 20, 2016

ところで、これが出来てしまうと今の Job API をどうしよう、というのはあります。

@mattn
Member
mattn commented Oct 20, 2016 edited

ご意見募集。どちらがいいでしょう。

(A) Job API を↑で入れ替える
(B) あくまでハイブリッド

@k-takata
Member

ループは do ... while の方がすっきりするかなという気もしますが、まあどちらでも。
Job APIはどうしますかね。

@mattn
Member
mattn commented Oct 20, 2016

入れ替えパッチ

https://gist.github.com/6a048122540cc1c1a4ebd9a373a725d4

引き続きご意見募集

@mattn
Member
mattn commented Oct 21, 2016

特に意見が出て来なければ、本日中に入れ替えパッチを出そうと思います。

@k-takata
Member

すでに終了しているプロセスに対して terminate_all を呼んだらどうなるんだろうと思いましたが、よく考えると今までと特に変わらないですかね?

@mattn
Member
mattn commented Oct 21, 2016

はい。
あと一応、前スレッド読み直しましたが JobObject を導入したのはプロセスの一括終了の為だけっぽいので入れ替えで要件は解決できると思います。

@mattn
Member
mattn commented Oct 21, 2016

vim/vim#1184 (comment)

vim-dev に流そうかと思ったけど元の issue へレス

@k-takata
Member

8.0.0048 でいったんマージされましたが、AppVeyorでfailするということで 8.0.0051 でrevertされました。
JobObject と併用の方がよいかもしれません。
原因がよく分かりませんが、子プロセスが JobObject を使った場合(py.exeとか)にうまく動かないとかでしょうか?
PRしてAppVeyor上でテストを流すようにすると確実ですね。

@mattn
Member
mattn commented Oct 28, 2016

Windows7 以下かどうかで投げ分けないといけないのかー。

@k-takata
Member

AssignProcessToJobObject() って特別な対策をしていなければ Win7 で失敗しませんでしたっけ?

@k-takata
Member

なので、元の処理の TerminateProcess を terminate_all に差し替えるだけでいいのではと思っているのですが。

@mattn
Member
mattn commented Oct 28, 2016 edited

あざます。テストパスしました。 vim/vim#1203

@k-takata
Member

私自身はまだWin7で動作確認していませんが、これでちゃんと動きました?

@mattn
Member
mattn commented Oct 28, 2016

はい。これで試しました。

let s:job = job_start(["cmd", "/c", "start notepad"])
call getchar()
call job_stop(s:job, "kill")
@k-takata
Member

それなら良かったです。 結果的にほぼ当初の想定通りになりましたね。 vim/vim#1184 (comment)

@h-east
Member
h-east commented Oct 29, 2016

patch 8.0.0054
vim/vim@fb63090

お疲れ様でした👍

@h-east h-east closed this Oct 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment