Thursday, November 29, 2018

Re: editors/vim and job control

On Thu, 29 Nov 2018 09:38:31 +0100
Olivier Taïbi <oli@olitb.net> wrote:

> 2) remove the check
> if (job_pid == getpgid(job_pid))
> from vim's mch_signal_job() in src/os_unix.c so that the whole group
> always gets a signal.

Thank you for finding the failing getpgid(). I suggest to remove the
check, like in the below diff. The process group id should equal the
process id after vim called setsid(2) or setpgid(2). I had helped to
remove a similar getpgid() call from meson:
https://github.com/mesonbuild/meson/commit/64906b0

I haven't used vim recently, but I tried to test the below diff with

:let j = job_start('sh -c "dd if=/dev/zero | cat | dd of=/dev/zero"')
:echo j
:let r = job_stop(j)

job_stop stopped the sh, dd, and cat processes. If I did kill -9 the
sh before job_stop, then job_stop still stopped dd and cat.

I also tried `make test` in /usr/ports/editors/vim, but it fails
because my terminal is smaller than 80x24. After I grow my terminal,
it fails with:

> make: don't know how to make writevimcmd (prerequisite of: test_arabic.res)

The documentation where it says, "The job will use the same terminal
as Vim.", is still wrong. Someone want to report a bug to vim?

Index: patches/patch-src_os_unix_c
===================================================================
RCS file: patches/patch-src_os_unix_c
diff -N patches/patch-src_os_unix_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_os_unix_c 29 Nov 2018 22:01:09 -0000
@@ -0,0 +1,22 @@
+$OpenBSD$
+
+Don't call getpgid(). It fails with EPERM in OpenBSD because setsid()
+had put job_pid in a different session. Go ahead and kill the process
+group that setsid() created.
+
+Index: src/os_unix.c
+--- src/os_unix.c.orig
++++ src/os_unix.c
+@@ -5871,11 +5871,7 @@ mch_signal_job(job_T *job, char_u *how)
+ return FAIL;
+
+ /* TODO: have an option to only kill the process, not the group? */
+- job_pid = job->jv_pid;
+-#ifdef HAVE_GETPGID
+- if (job_pid == getpgid(job_pid))
+- job_pid = -job_pid;
+-

No comments:

Post a Comment