vim-jp / issues

有志で既知のバグや要望を検討・管理し、オフィシャルへの還元をしていきます。
https://vim-jp.org/
342 stars 11 forks source link

channel機能を叩く #819

Closed k-takata closed 8 years ago

k-takata commented 8 years ago

Vimに導入されたばかりのchannel機能について議論しましょう。(関連 #820)

次のリリースが出たら、このissueは閉じます。

h-east commented 8 years ago

@k-takata :+1:

6日ほど前にtwitterで呟いたやつです。

Vimのchannel-demoでサーバーから ["ex","echo 'foo'"] や ["ex","colorscheme"] を送ったら即表示更新されるのに、["ex","set nu"] や ["ex","set nu?"] は更新されないのはなんでですか?

["redraw",""] を送れば更新されるけど違いが分からない。

再現方法: (:h channel-demo を見ればだいたい分かる) (1) 端末を開いてデモサーバーを起動。

$ python3 demoserver.py

(2) もう1つの端末でVimを起動してch_openする。

$ vim -Nu NONE
:let handle = ch_open('localhost:8765')

(3) デモサーバーに以下の文字を入力してVimの画面表示を確認する。

["ex","echo 'foo'"]

→コマンドラインにfooと即表示される。

["ex","colorscheme"]

→コマンドラインにdefaultと即表示される。

["ex","set nu"]

→表示更新されない。

["redraw",""]

→表示更新された。(1行目左側に1が表示される)

["ex","set nu?"]

→表示更新されない。

["redraw",""]

→表示更新された。(コマンドラインにnumberが表示される)

違いは何なのだろう。 環境: Vim 7.4.1301 (Huge) on fedora via PuTTY

k-takata commented 8 years ago

Windowsで、src/testdir/test_channel.vim を実行すると、python.exe が残ったままになるという問題があるので、 https://bitbucket.org/k_takata/vim-ktakata-mq/src/2645f8ca45792207712f57fb5ff2d5fad41bdec3/fix-test_channel-vim-on-windows.patch?fileviewer=file-view-default を当ててみたところ、今度は、

From test_channel.vim:
Found errors in Test_server_crash():
function Test_server_crash[1]..<SNR>7_run_server line 38: 'Caught exception: Vim(sleep):E896: read from channel: The system cannot find the file specified.'

というログが残って、テストに失敗する問題が起きています。(Win32 GUIで確認。CUIは未確認。)

koron commented 8 years ago

Windowsで、src/testdir/test_channel.vim を実行すると、python.exe が残ったままになる

それはどちらかというと job の問題では?

ichizok commented 8 years ago

*BSD だと test_channel.vim の Test_connect_waittime() で失敗します。 FreeBSD 10.2, OpenBSD 5.8 で確認。

Non-blocking socket が listen されてないローカルポートに connect() すると ECONNREFUSED エラーになります。

BSD 以外 (Linux, OS X, Solaris で確認) だと EWOULDBLOCKEINPROGRESS で「いったんエラーで戻って、connect 処理は継続する」という動作になるようですが...

k-takata commented 8 years ago

それはどちらかというと job の問題では?

そこは今は問題にしていません。上記のパッチにあるように、job_stop()"kill"を指定すれば強制終了できます。

Vim(sleep):E896: read from channel: The system cannot find the file specified.

が発生することを問題視しています。

koron commented 8 years ago

ああ、なるほど。

syohex commented 8 years ago

*BSD だと test_channel.vim の Test_connect_waittime() で失敗します。 FreeBSD 10.2, OpenBSD 5.8 で確認。

Non-blocking socket が listen されてないローカルポートに connect() すると ECONNREFUSED エラーになります。

BSD 以外 (Linux, OS X, Solaris で確認) だと EWOULDBLOCK や EINPROGRESS で「いったんエラーで戻って、connect 処理は継続する」という動作になるようですが...

NetBSDでは, EINPROGRESSが返ってくるため, すべての *BSDというわけではないと思います. (Vimのテストが今ひとつよくわかっていませんが), NetBSDでテストを行ったところ, FreeBSDでテストしたときのように例外が上がるということはありませんでした(成功したのか失敗したのかもよくわかっていませんが, 以下のダンプが出るところまではテストは進んでいます)

Traceback (most recent call last):
  File "/usr/pkg/lib/python2.7/SocketServer.py", line 599, in process_request_thread
    self.finish_request(request, client_address)
  File "/usr/pkg/lib/python2.7/SocketServer.py", line 334, in finish_request
    self.RequestHandlerClass(request, client_address, self)
  File "/usr/pkg/lib/python2.7/SocketServer.py", line 655, in __init__
    self.handle()
  File "test_channel.py", line 149, in handle
    42 / 0
ichizok commented 8 years ago

NetBSDでは, EINPROGRESSが返ってくるため, すべての *BSDというわけではないと思います.

DragonFlyBSD 4.4 も EINPROGRESS が返ることを確認しました。すみません ; このコードで検証しています。 https://gist.github.com/ichizok/04a7695a3a576f352c77

ynkdir commented 8 years ago

Windows これで落ちます。

function! F()
  let h = job_start('py test_channel.py')
  let port = []
  while 1
    try
      let port = readfile('Xportnr')
    catch
    endtry
    if len(port) >= 1
      break
    endif
    sleep 200m
  endwhile
  call delete('Xportnr')
  let c = ch_open('localhost:' . port[0])
  call ch_sendexpr(c, 'hello!')
  call job_stop(h)
endfunction

while 1
  call F()
endwhile
mattn commented 8 years ago

@ynkdir 再現しました。

ynkdir commented 8 years ago

パッチ投げました。 https://groups.google.com/d/msg/vim_dev/EShklPqqY-8/Cs2aNADHDgAJ

WM_NETBEANS の処理では throw しない。 クラッシュ以外にも限らず全然関係ないコマンドでソケットエラーが飛ぶのはどうかというのもあります。 エラーメッセージは表示されます。 これでエラーが出るときはたぶんスクリプトかサーバーに問題があるときだけなのでエラーメッセージは出たほうがいいでしょう。 WM_NETBEANS があるけどデータはすでに read した後、という状況はエラーにはならないので WM_NETBEANS がメッセージキューに残る問題は問題なさげ。

mattn commented 8 years ago

:+1:

k-takata commented 8 years ago

Win32 CUIで test_channel.vim がpassしません。

From test_channel.vim:
Found errors in Test_channel_handler():
function Test_channel_handler[2]..<SNR>3_run_server[36]..<SNR>3_channel_handler line 15: Expected 'we did call you' but got 'we called you'
function Test_channel_handler[4]..<SNR>3_run_server[36]..<SNR>3_channel_handler line 15: Expected 'we did call you' but got 'we called you'
Found errors in Test_close_handle():
function Test_close_handle[1]..<SNR>3_run_server[36]..<SNR>3_close_handle line 4: Expected 'what?' but got ''
Found errors in Test_communicate():
function Test_communicate[1]..<SNR>3_run_server[36]..<SNR>3_communicate line 26: 's:responseHandle was not set'
function Test_communicate[1]..<SNR>3_run_server[36]..<SNR>3_communicate line 30: Expected 'got it' but got ''
function Test_communicate[1]..<SNR>3_run_server line 38: 'Caught exception: Vim(unlet):E108: No such variable: "s:responseHandle"'
Found errors in Test_unlet_handle():
function Test_unlet_handle[1]..<SNR>3_run_server[36]..<SNR>3_unlet_handle line 4: Expected 'what?' but got ''

詳細未確認。

k-takata commented 8 years ago

Win32 CUI 7.4.1362 でも変化なし。

From test_channel.vim:
Found errors in Test_channel_handler():
function Test_channel_handler[3]..<SNR>3_run_server[43]..<SNR>3_channel_handler line 15: Expected 'we did call you' but got 'we called you'
function Test_channel_handler[5]..<SNR>3_run_server[43]..<SNR>3_channel_handler line 15: Expected 'we did call you' but got 'we called you'
Found errors in Test_close_handle():
function Test_close_handle[2]..<SNR>3_run_server[43]..<SNR>3_close_handle line 4: Expected 'what?' but got ''
Found errors in Test_communicate():
function Test_communicate[2]..<SNR>3_run_server[43]..<SNR>3_communicate line 26: 's:responseHandle was not set'
function Test_communicate[2]..<SNR>3_run_server[43]..<SNR>3_communicate line 30: Expected 'got it' but got ''
function Test_communicate[2]..<SNR>3_run_server line 45: 'Caught exception: Vim(unlet):E108: No such variable: "s:responseHandle"'
Found errors in Test_unlet_handle():
function Test_unlet_handle[2]..<SNR>3_run_server[43]..<SNR>3_unlet_handle line 4: Expected 'what?' but got ''

しかも、python.exeが大量に残ったままになっています。

k-takata commented 8 years ago

Win32 GUIでは特に問題なし。python.exeも残っていません。

mattn commented 8 years ago

うわー

mattn commented 8 years ago

とりあえず、こうすれば python は残らない事が分かった。

diff --git a/src/testdir/test_channel.py b/src/testdir/test_channel.py
index ec231e8..dc4a79f 100644
--- a/src/testdir/test_channel.py
+++ b/src/testdir/test_channel.py
@@ -192,3 +192,8 @@ if __name__ == "__main__":

     # Main thread terminates, but the server continues running
     # until server.shutdown() is called.
+    try:
+      while server_thread.isAlive(): 
+        server_thread.join(1)
+    except (KeyboardInterrupt, SystemExit):
+      server.shutdown()
mattn commented 8 years ago

windows の場合は test_channel.py を起動している時は kill じゃないと殺せなかった。引数無しだと殺せた。

mattn commented 8 years ago

threading 使ってるとプロセス残るのが分かったのでパッチ送付。

ynkdir commented 8 years ago

windows vim.exe で :sleep でチャンネル読み込み発動してないです。

mattn commented 8 years ago

あー。WaitForXX とか他の方法考えた方がいいすね。

mattn commented 8 years ago

ん? ex_sleep -> ui_delay ですが、GUI だと Sleep ですね。なんで GUI は成功するんだろう。

ynkdir commented 8 years ago

ui_breakcheck() でメッセージループを回してるので なのでおそらくパイプを sleep で待つのは GUI でも動かないはずです

mattn commented 8 years ago

んー。do_sleep のループの中で channel_handle_events してみたんですが

function Test_channel_handler[3]..<SNR>3_run_server line 45: 'Caught exception: Vim(call):E906: not an open channel'
function Test_channel_handler[5]..<SNR>3_run_server line 45: 'Caught exception: Vim(call):E906: not an open channel'
mattn commented 8 years ago

現在のパッチです。

diff --git a/src/ex_docmd.c b/src/ex_docmd.c
index 1321b7f..bce7eaa 100644
--- a/src/ex_docmd.c
+++ b/src/ex_docmd.c
@@ -8910,6 +8910,10 @@ do_sleep(long msec)
     * may occur when running a test case. */
    parse_queued_messages();
 #endif
+
+#ifdef FEAT_CHANNEL
+   channel_handle_events();
+#endif
     }
 }

diff --git a/src/testdir/runtest.vim b/src/testdir/runtest.vim
index 2d5e21c..3ea73ae 100644
--- a/src/testdir/runtest.vim
+++ b/src/testdir/runtest.vim
@@ -93,6 +93,9 @@ redir @q
 silent function /^Test_
 redir END
 let s:tests = split(substitute(@q, 'function \(\k*()\)', '\1', 'g'))
+if argc() > 1
+  let s:tests = filter(s:tests, 'v:val=~argv(1)')
+endif

 " Execute the tests in alphabetical order.
 for s:test in sort(s:tests)
diff --git a/src/testdir/test_channel.py b/src/testdir/test_channel.py
index ec231e8..c27b39e 100644
--- a/src/testdir/test_channel.py
+++ b/src/testdir/test_channel.py
@@ -192,3 +192,8 @@ if __name__ == "__main__":

     # Main thread terminates, but the server continues running
     # until server.shutdown() is called.
+    try:
+      while server_thread.isAlive():
+        server_thread.join(1)
+    except (KeyboardInterrupt, SystemExit):
+      server.shutdown()
k-takata commented 8 years ago

7.4.1370 https://groups.google.com/d/topic/vim_dev/FO8Le61YB7Y/discussion

ynkdir commented 8 years ago

channel_handle_events() と parse_queued_messages() の呼ぶ順番。 1/2 で channel_handle_events() が動かない。 (CUI) channel_read() をいきなり呼ぶと WSAEWOULDBLOCK エラーでソケットを閉じてしまう。

これでいいかはわかりませんが いちおう動くと思います

diff --git a/src/channel.c b/src/channel.c
index aa478f1..266b647 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -1887,11 +1887,7 @@ channel_handle_events(void)
 {
     channel_T  *channel;
     int        part;
-    static int loop = 0;
-
-    /* Skip heavily polling */
-    if (loop++ % 2)
-   return;
+    sock_T fd;

     for (channel = first_channel; channel != NULL; channel = channel->ch_next)
     {
@@ -1907,7 +1903,11 @@ channel_handle_events(void)
    part = PART_SOCK;
 #   endif
 #  endif
-   channel_read(channel, part, "channel_handle_events");
+   {
+       fd = channel->ch_part[part].ch_fd;
+       if (fd != INVALID_FD && channel_wait(channel, fd, 0) == OK)
+       channel_read(channel, part, "channel_handle_events");
+   }
     }
 }
 # endif
diff --git a/src/ex_docmd.c b/src/ex_docmd.c
index 1321b7f..5b4c6ed 100644
--- a/src/ex_docmd.c
+++ b/src/ex_docmd.c
@@ -8904,6 +8904,9 @@ do_sleep(long msec)
     {
    ui_delay(msec - done > 1000L ? 1000L : msec - done, TRUE);
    ui_breakcheck();
+#ifdef FEAT_CHANNEL
+   channel_handle_events();
+#endif
 #ifdef MESSAGE_QUEUE
    /* Process the netbeans and clientserver messages that may have been
     * received in the call to ui_breakcheck() when the GUI is in use. This
mattn commented 8 years ago

間引かないとカーソル移動が重たくなるんですよねー。つらい。 ありがとうございます。試してみます。(okそうならパッチ送付お願いします)

mattn commented 8 years ago

んー。手元だとテスト通らないですね。

diff --git a/src/channel.c b/src/channel.c
index aa478f1..4a20b9d 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -1887,11 +1887,7 @@ channel_handle_events(void)
 {
     channel_T  *channel;
     int        part;
-    static int loop = 0;
-
-    /* Skip heavily polling */
-    if (loop++ % 2)
-   return;
+    sock_T fd;

     for (channel = first_channel; channel != NULL; channel = channel->ch_next)
     {
@@ -1907,7 +1903,9 @@ channel_handle_events(void)
    part = PART_SOCK;
 #   endif
 #  endif
-   channel_read(channel, part, "channel_handle_events");
+   fd = channel->ch_part[part].ch_fd;
+   if (fd != INVALID_FD && channel_wait(channel, fd, 0) == OK)
+       channel_read(channel, part, "channel_handle_events");
     }
 }
 # endif
diff --git a/src/ex_docmd.c b/src/ex_docmd.c
index 1321b7f..5b4c6ed 100644
--- a/src/ex_docmd.c
+++ b/src/ex_docmd.c
@@ -8904,6 +8904,9 @@ do_sleep(long msec)
     {
    ui_delay(msec - done > 1000L ? 1000L : msec - done, TRUE);
    ui_breakcheck();
+#ifdef FEAT_CHANNEL
+   channel_handle_events();
+#endif
 #ifdef MESSAGE_QUEUE
    /* Process the netbeans and clientserver messages that may have been
     * received in the call to ui_breakcheck() when the GUI is in use. This
ynkdir commented 8 years ago
+   {
+       fd = channel->ch_part[part].ch_fd;
+       if (fd != INVALID_FD && channel_wait(channel, fd, 0) == OK)
+       channel_read(channel, part, "channel_handle_events");
+   }

カッコが抜けてます。

ynkdir commented 8 years ago

間引かないとカーソル移動が重たくなるんですよねー。つらい。

なるほどー。むつかしい。

mattn commented 8 years ago

おーほんとだw

すいません。そしてテストパスしました。

mattn commented 8 years ago

ちなみに channel_read の頭で channel_wait 相当をやってるので channel_read でもいいかなーと思うのですがどうでしょう。

mattn commented 8 years ago
diff --git a/src/channel.c b/src/channel.c
index aa478f1..84356c9 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -1582,7 +1582,7 @@ channel_wait(channel_T *channel, sock_T fd, int timeout)
                                 && nread > 0)
        return OK;
        diff = deadline - GetTickCount();
-       if (diff < 0)
+       if (diff <= 0)
        break;
        /* Wait for 5 msec.
         * TODO: increase the sleep time when looping more often */
@@ -1887,11 +1887,7 @@ channel_handle_events(void)
 {
     channel_T  *channel;
     int        part;
-    static int loop = 0;
-
-    /* Skip heavily polling */
-    if (loop++ % 2)
-   return;
+    sock_T fd;

     for (channel = first_channel; channel != NULL; channel = channel->ch_next)
     {
@@ -1907,7 +1903,11 @@ channel_handle_events(void)
    part = PART_SOCK;
 #   endif
 #  endif
-   channel_read(channel, part, "channel_handle_events");
+   {
+       fd = channel->ch_part[part].ch_fd;
+       if (fd != INVALID_FD && channel_wait(channel, fd, 0) == OK)
+       channel_read(channel, part, "channel_handle_events");
+   }
     }
 }
 # endif
diff --git a/src/ex_docmd.c b/src/ex_docmd.c
index 1321b7f..5b4c6ed 100644
--- a/src/ex_docmd.c
+++ b/src/ex_docmd.c
@@ -8904,6 +8904,9 @@ do_sleep(long msec)
     {
    ui_delay(msec - done > 1000L ? 1000L : msec - done, TRUE);
    ui_breakcheck();
+#ifdef FEAT_CHANNEL
+   channel_handle_events();
+#endif
 #ifdef MESSAGE_QUEUE
    /* Process the netbeans and clientserver messages that may have been
     * received in the call to ui_breakcheck() when the GUI is in use. This

これで改善しました。

ynkdir commented 8 years ago

channel_read() の中で channel_wait() が FAIL を返すとエラー扱いになってチャンネルが閉じられます。 GUI でソケットの場合だけ特別扱いで入力がなくてもエラーにしないようになってます。

mattn commented 8 years ago

https://github.com/mattn/vim-tail

これ使って編集しながらでも問題なく tail 出来るレベルになりました。:+1: パッチの送付よろしくお願いします。

ynkdir commented 8 years ago

投げました https://groups.google.com/d/msg/vim_dev/ncRD_JnVaR4/MC4Vr7VDEQAJ

h-east commented 8 years ago

そして取り込まれました:+1: Patch 7.4.1379 https://groups.google.com/d/topic/vim_dev/NRhVGNPv3To/discussion

ynkdir commented 8 years ago

Windows の実装は 7.4.1404 で WSAAsyncSelect() をやめて入力待ちのときに 100ms (適当) 毎にポーリングする形になりました。 ウェイト時間が問題になるようなら Overlapped I/O に切り替えるしかないと思います。

CUI はあとは Test_exit_callback() が失敗してます。

mattn commented 8 years ago

僕が投げたパッチだと即検知してたんですが、Bram が戻してしまったからですかね。 10秒間隔でしかチェックされないので 10ms では到底検知出来ないすね。

mattn commented 8 years ago

ちがうな。死ぬ継起が無いわ。

mattn commented 8 years ago

あれ、でもGUIだとパスする

mattn commented 8 years ago

これで通る様ですね。

diff --git a/src/testdir/test_channel.vim b/src/testdir/test_channel.vim
index 69922b1..d328742 100644
--- a/src/testdir/test_channel.vim
+++ b/src/testdir/test_channel.vim
@@ -483,6 +483,8 @@ func Test_exit_callback()
   if has('job')
     call s:run_server('s:test_exit_callback')

+    sleep 1
+    call job_stop(s:exit_job, "kill")
     " the job may take a little while to exit
     sleep 50m
mattn commented 8 years ago

https://groups.google.com/forum/#!topic/vim_dev/JhaQGrrOTnk

ynkdir commented 8 years ago

CUI だと job_stop(job, 'hup') が効かないのか。 AttachConsole() が失敗している。

ynkdir commented 8 years ago

こんなんどうですか。 Vim と同じコンソールで動くとまずいんでしたっけ。

diff --git a/src/os_win32.c b/src/os_win32.c
index e93e6d0..89921e0 100644
--- a/src/os_win32.c
+++ b/src/os_win32.c
@@ -5071,8 +5071,10 @@ mch_start_job(char *cmd, job_T *job, jobopt_T *options)
     if (!vim_create_process(cmd, TRUE,
        CREATE_SUSPENDED |
        CREATE_DEFAULT_ERROR_MODE |
-       CREATE_NEW_PROCESS_GROUP |
-       CREATE_NEW_CONSOLE,
+#ifdef FEAT_GUI_W32
+       CREATE_NEW_CONSOLE |
+#endif
+       CREATE_NEW_PROCESS_GROUP,
        &si, &pi))
     {
    CloseHandle(jo);
@@ -5152,13 +5154,17 @@ mch_stop_job(job_T *job, char_u *how)
        return TerminateProcess(job->jv_proc_info.hProcess, 0) ? OK : FAIL;
     }

+#ifdef FEAT_GUI_W32
     if (!AttachConsole(job->jv_proc_info.dwProcessId))
    return FAIL;
+#endif
     ret = GenerateConsoleCtrlEvent(
        ctrl_c ? CTRL_C_EVENT : CTRL_BREAK_EVENT,
        job->jv_proc_info.dwProcessId)
    ? OK : FAIL;
+#ifdef FEAT_GUI_W32
     FreeConsole();
+#endif
     return ret;
 }
k-takata commented 8 years ago

7.4.1370 以降もpython.exeが大量に残ったままになる現象が起きていてなぜだろうと思っていましたが、なるほどそういうことでしたか。

Vim と同じコンソールで動くとまずいんでしたっけ。

問題ないのではないでしょうか。

ynkdir commented 8 years ago

投げてみました。

ynkdir commented 8 years ago

じつは CreateProcess() に CREATE_NO_WINDOW つけても大丈夫ぽい

ynkdir commented 8 years ago

じつは CreateProcess() に CREATE_NO_WINDOW つけても大丈夫ぽい

うそでした。GUI は OK で CUI は job_stop() が効かなくなる。