I found memory leakage in term_and_job_init() on Win32.
I also added a workaround when AssignProcessToJobObject() failed. This the same
as mch_job_start() in os_win32.c and would be useful on Windows 7 or earlier.
Please check the attached patch.
Regards,
Ken Takata
And here is another patch written by mattn, which fixes refcount problem.
Jobs were freed unexpectedly by GC.
Please apply this after my previous patch to avoid conflicts. (That's why I
send this patch, not from mattn.)
Regards,
Ken Takata
https://gist.github.com/ichizok/0f5559a771ae40c15616e20cd3515d2b
(this includes above fix-terminal-refcount.patch)
* Add set_ref_in_term() to update copyID of job of terminal
Ozaki Kiichi
2017/7/28 Fri 4:47:08 UTC+9 Bram Moolenaar wrote:
> Ken Takata wrote:
>
> > I found memory leakage in term_and_job_init() on Win32.
> > I also added a workaround when AssignProcessToJobObject() failed.
> > This the same
> > as mch_job_start() in os_win32.c and would be useful on Windows 7 or earlier.
> > Please check the attached patch.
>
> Thanks. Looks like spawn_config also needs to be freed after "failed:".
>
> > @@ -1227,7 +1228,12 @@ term_and_job_init(term_T *term, int rows
> > goto failed;
> >
> > if (!AssignProcessToJobObject(jo, child_process_handle))
> > - goto failed;
> > + {
> > + /* if failing, switch the way to terminate
> > + * process with TerminateProcess. */
> > + CloseHandle(jo);
> > + jo = NULL;
> > + }
>
> Shouldn't it goto failed here?
No, it shouldn't. This is the same as mch_job_start() in os_win32.c:
https://github.com/vim/vim/blob/67883b4909d0e9d4c024beb18f02750c6f7e3069/src/os_win32.c#L5098-L5104
If the process has been already assigned to a job object,
AssignProcessToJobObject() fails on Windows 7 or earlier, because a process
can be assigned to only one job object on those OSes.
In that case, it should fall back to a way without using a job object.
Regards,
Ken Takata