Patch 8.0.0087
Problem: When the channel callback gets job info the job may already have
been deleted. (lifepillar)
Solution: Do not delete the job when the channel is still useful. (ichizok,
closes #1242, closes #1245)
Files: src/channel.c, src/eval.c, src/os_unix.c, src/os_win32.c,
src/structs.h, src/testdir/test_channel.vim
*** ../vim-8.0.0086/src/channel.c 2016-11-12 21:12:48.538182233 +0100
--- src/channel.c 2016-11-17 17:13:15.080933875 +0100
***************
*** 4433,4451 ****
}
}
static void
job_cleanup(job_T *job)
{
if (job->jv_status != JOB_ENDED)
return;
if (job->jv_exit_cb != NULL)
{
typval_T argv[3];
typval_T rettv;
int dummy;
! /* invoke the exit callback; make sure the refcount is > 0 */
++job->jv_refcount;
argv[0].v_type = VAR_JOB;
argv[0].vval.v_job = job;
--- 4433,4498 ----
}
}
+ #if defined(EXITFREE) || defined(PROTO)
+ void
+ job_free_all(void)
+ {
+ while (first_job != NULL)
+ job_free(first_job);
+ }
+ #endif
+
+ /*
+ * Return TRUE if we need to check if the process of "job" has ended.
+ */
+ static int
+ job_need_end_check(job_T *job)
+ {
+ return job->jv_status == JOB_STARTED
+ && (job->jv_stoponexit != NULL || job->jv_exit_cb != NULL);
+ }
+
+ /*
+ * Return TRUE if the channel of "job" is still useful.
+ */
+ static int
+ job_channel_still_useful(job_T *job)
+ {
+ return job->jv_channel != NULL && channel_still_useful(job->jv_channel);
+ }
+
+ /*
+ * Return TRUE if the job should not be freed yet. Do not free the job when
+ * it has not ended yet and there is a "stoponexit" flag, an exit callback
+ * or when the associated channel will do something with the job output.
+ */
+ static int
+ job_still_useful(job_T *job)
+ {
+ return job_need_end_check(job) || job_channel_still_useful(job);
+ }
+
+ /*
+ * NOTE: Must call job_cleanup() only once right after the status of "job"
+ * changed to JOB_ENDED (i.e. after job_status() returned "dead" first or
+ * mch_detect_ended_job() returned non-NULL).
+ */
static void
job_cleanup(job_T *job)
{
if (job->jv_status != JOB_ENDED)
return;
+ /* Ready to cleanup the job. */
+ job->jv_status = JOB_FINISHED;
+
if (job->jv_exit_cb != NULL)
{
typval_T argv[3];
typval_T rettv;
int dummy;
! /* Invoke the exit callback. Make sure the refcount is > 0. */
++job->jv_refcount;
argv[0].v_type = VAR_JOB;
argv[0].vval.v_job = job;
***************
*** 4458,4499 ****
--job->jv_refcount;
channel_need_redraw = TRUE;
}
! if (job->jv_refcount == 0)
! {
! /* The job was already unreferenced, now that it ended it can be
! * freed. Careful: caller must not use "job" after this! */
job_free(job);
}
}
- #if defined(EXITFREE) || defined(PROTO)
- void
- job_free_all(void)
- {
- while (first_job != NULL)
- job_free(first_job);
- }
- #endif
-
- /*
- * Return TRUE if the job should not be freed yet. Do not free the job when
- * it has not ended yet and there is a "stoponexit" flag, an exit callback
- * or when the associated channel will do something with the job output.
- */
- static int
- job_still_useful(job_T *job)
- {
- return (job->jv_stoponexit != NULL || job->jv_exit_cb != NULL
- || (job->jv_channel != NULL
- && channel_still_useful(job->jv_channel)));
- }
-
- static int
- job_still_alive(job_T *job)
- {
- return (job->jv_status == JOB_STARTED) && job_still_useful(job);
- }
-
/*
* Mark references in jobs that are still useful.
*/
--- 4505,4522 ----
--job->jv_refcount;
channel_need_redraw = TRUE;
}
!
! /* Do not free the job in case the close callback of the associated
channel
! * isn't invoked yet and may get information by job_info(). */
! if (job->jv_refcount == 0 && !job_channel_still_useful(job))
! {
! /* The job was already unreferenced and the associated channel was
! * detached, now that it ended it can be freed. Careful: caller must
! * not use "job" after this! */
job_free(job);
}
}
/*
* Mark references in jobs that are still useful.
*/
***************
*** 4505,4511 ****
typval_T tv;
for (job = first_job; job != NULL; job = job->jv_next)
! if (job_still_alive(job))
{
tv.v_type = VAR_JOB;
tv.vval.v_job = job;
--- 4528,4534 ----
typval_T tv;
for (job = first_job; job != NULL; job = job->jv_next)
! if (job_still_useful(job))
{
tv.v_type = VAR_JOB;
tv.vval.v_job = job;
***************
*** 4514,4539 ****
return abort;
}
void
job_unref(job_T *job)
{
if (job != NULL && --job->jv_refcount <= 0)
{
! /* Do not free the job when it has not ended yet and there is a
! * "stoponexit" flag or an exit callback. */
! if (!job_still_alive(job))
! {
! job_free(job);
! }
! else if (job->jv_channel != NULL
! && !channel_still_useful(job->jv_channel))
! {
! /* Do remove the link to the channel, otherwise it hangs
! * around until Vim exits. See job_free() for refcount. */
! ch_log(job->jv_channel, "detaching channel from job");
! job->jv_channel->ch_job = NULL;
! channel_unref(job->jv_channel);
! job->jv_channel = NULL;
}
}
}
--- 4537,4569 ----
return abort;
}
+ /*
+ * Dereference "job". Note that after this "job" may have been freed.
+ */
void
job_unref(job_T *job)
{
if (job != NULL && --job->jv_refcount <= 0)
{
! /* Do not free the job if there is a channel where the close callback
! * may get the job info. */
! if (!job_channel_still_useful(job))
! {
! /* Do not free the job when it has not ended yet and there is a
! * "stoponexit" flag or an exit callback. */
! if (!job_need_end_check(job))
! {
! job_free(job);
! }
! else if (job->jv_channel != NULL)
! {
! /* Do remove the link to the channel, otherwise it hangs
! * around until Vim exits. See job_free() for refcount. */
! ch_log(job->jv_channel, "detaching channel from job");
! job->jv_channel->ch_job = NULL;
! channel_unref(job->jv_channel);
! job->jv_channel = NULL;
! }
}
}
}
***************
*** 4546,4552 ****
for (job = first_job; job != NULL; job = job->jv_next)
if ((job->jv_copyID & mask) != (copyID & mask)
! && !job_still_alive(job))
{
/* Free the channel and ordinary items it contains, but don't
* recurse into Lists, Dictionaries etc. */
--- 4576,4582 ----
for (job = first_job; job != NULL; job = job->jv_next)
if ((job->jv_copyID & mask) != (copyID & mask)
! && !job_still_useful(job))
{
/* Free the channel and ordinary items it contains, but don't
* recurse into Lists, Dictionaries etc. */
***************
*** 4566,4572 ****
{
job_next = job->jv_next;
if ((job->jv_copyID & mask) != (copyID & mask)
! && !job_still_alive(job))
{
/* Free the job struct itself. */
job_free_job(job);
--- 4596,4602 ----
{
job_next = job->jv_next;
if ((job->jv_copyID & mask) != (copyID & mask)
! && !job_still_useful(job))
{
/* Free the job struct itself. */
job_free_job(job);
***************
*** 4660,4667 ****
/* Only should check if the channel has been closed, if the channel is
* open the job won't exit. */
if (job->jv_status == JOB_STARTED && job->jv_exit_cb != NULL
! && (job->jv_channel == NULL
! || !channel_still_useful(job->jv_channel)))
return TRUE;
return FALSE;
}
--- 4690,4696 ----
/* Only should check if the channel has been closed, if the channel is
* open the job won't exit. */
if (job->jv_status == JOB_STARTED && job->jv_exit_cb != NULL
! && !job_channel_still_useful(job))
return TRUE;
return FALSE;
}
***************
*** 4676,4689 ****
{
int i;
for (i = 0; i < MAX_CHECK_ENDED; ++i)
{
job_T *job = mch_detect_ended_job(first_job);
if (job == NULL)
break;
! if (job_still_useful(job))
! job_cleanup(job); /* may free "job" */
}
if (channel_need_redraw)
--- 4705,4722 ----
{
int i;
+ if (first_job == NULL)
+ return;
+
for (i = 0; i < MAX_CHECK_ENDED; ++i)
{
+ /* NOTE: mch_detect_ended_job() must only return a job of which the
+ * status was just set to JOB_ENDED. */
job_T *job = mch_detect_ended_job(first_job);
if (job == NULL)
break;
! job_cleanup(job); /* may free "job" */
}
if (channel_need_redraw)
***************
*** 4897,4903 ****
{
char *result;
! if (job->jv_status == JOB_ENDED)
/* No need to check, dead is dead. */
result = "dead";
else if (job->jv_status == JOB_FAILED)
--- 4930,4936 ----
{
char *result;
! if (job->jv_status >= JOB_ENDED)
/* No need to check, dead is dead. */
result = "dead";
else if (job->jv_status == JOB_FAILED)
*** ../vim-8.0.0086/src/eval.c 2016-11-14 21:49:57.080090226 +0100
--- src/eval.c 2016-11-17 16:09:44.478455636 +0100
***************
*** 7290,7296 ****
if (job == NULL)
return (char_u *)"no process";
status = job->jv_status == JOB_FAILED ? "fail"
! : job->jv_status == JOB_ENDED ? "dead"
: "run";
# ifdef UNIX
vim_snprintf((char *)buf, NUMBUFLEN,
--- 7290,7296 ----
if (job == NULL)
return (char_u *)"no process";
status = job->jv_status == JOB_FAILED ? "fail"
! : job->jv_status >= JOB_ENDED ? "dead"
: "run";
# ifdef UNIX
vim_snprintf((char *)buf, NUMBUFLEN,
*** ../vim-8.0.0086/src/os_unix.c 2016-11-12 21:12:48.538182233 +0100
--- src/os_unix.c 2016-11-17 16:09:44.478455636 +0100
***************
*** 5354,5360 ****
return "run";
return_dead:
! if (job->jv_status != JOB_ENDED)
{
ch_log(job->jv_channel, "Job ended");
job->jv_status = JOB_ENDED;
--- 5354,5360 ----
return "run";
return_dead:
! if (job->jv_status < JOB_ENDED)
{
ch_log(job->jv_channel, "Job ended");
job->jv_status = JOB_ENDED;
***************
*** 5398,5404 ****
job->jv_exitval = WEXITSTATUS(status);
else if (WIFSIGNALED(status))
job->jv_exitval = -1;
! if (job->jv_status != JOB_ENDED)
{
ch_log(job->jv_channel, "Job ended");
job->jv_status = JOB_ENDED;
--- 5398,5404 ----
job->jv_exitval = WEXITSTATUS(status);
else if (WIFSIGNALED(status))
job->jv_exitval = -1;
! if (job->jv_status < JOB_ENDED)
{
ch_log(job->jv_channel, "Job ended");
job->jv_status = JOB_ENDED;
*** ../vim-8.0.0086/src/os_win32.c 2016-10-29 14:54:56.628135821 +0200
--- src/os_win32.c 2016-11-17 16:09:44.478455636 +0100
***************
*** 4978,4984 ****
|| dwExitCode != STILL_ACTIVE)
{
job->jv_exitval = (int)dwExitCode;
! if (job->jv_status != JOB_ENDED)
{
ch_log(job->jv_channel, "Job ended");
job->jv_status = JOB_ENDED;
--- 4978,4984 ----
|| dwExitCode != STILL_ACTIVE)
{
job->jv_exitval = (int)dwExitCode;
! if (job->jv_status < JOB_ENDED)
{
ch_log(job->jv_channel, "Job ended");
job->jv_status = JOB_ENDED;
*** ../vim-8.0.0086/src/structs.h 2016-11-10 20:20:01.870602701 +0100
--- src/structs.h 2016-11-17 16:17:49.727271673 +0100
***************
*** 1421,1431 ****
dict_T *pt_dict; /* dict for "self" */
};
typedef enum
{
JOB_FAILED,
JOB_STARTED,
! JOB_ENDED
} jobstatus_T;
/*
--- 1421,1433 ----
dict_T *pt_dict; /* dict for "self" */
};
+ /* Status of a job. Order matters! */
typedef enum
{
JOB_FAILED,
JOB_STARTED,
! JOB_ENDED, /* detected job done */
! JOB_FINISHED /* job done and cleanup done */
} jobstatus_T;
/*
*** ../vim-8.0.0086/src/testdir/test_channel.vim 2016-10-27
20:00:03.665357405 +0200
--- src/testdir/test_channel.vim 2016-11-17 16:07:29.307354887 +0100
***************
*** 1232,1237 ****
--- 1232,1263 ----
endtry
endfunc
+ func Test_close_and_exit_cb()
+ if !has('job')
+ return
+ endif
+ call ch_log('Test_close_and_exit_cb')
+
+ let dict = {'ret': {}}
+ func dict.close_cb(ch) dict
+ let self.ret['close_cb'] = job_status(ch_getjob(a:ch))
+ endfunc
+ func dict.exit_cb(job, status) dict
+ let self.ret['exit_cb'] = job_status(a:job)
+ endfunc
+
+ let g:job = job_start('echo', {
+ \ 'close_cb': dict.close_cb,
+ \ 'exit_cb': dict.exit_cb,
+ \ })
+ call assert_equal('run', job_status(g:job))
+ unlet g:job
+ call WaitFor('len(dict.ret) >= 2')
+ call assert_equal(2, len(dict.ret))
+ call assert_match('^\%(dead\|run\)', dict.ret['close_cb'])
+ call assert_equal('dead', dict.ret['exit_cb'])
+ endfunc
+
""""""""""
let g:Ch_unletResponse = ''
*** ../vim-8.0.0086/src/version.c 2016-11-15 21:16:46.754453019 +0100
--- src/version.c 2016-11-17 16:10:21.326213761 +0100
***************
*** 766,767 ****
--- 766,769 ----
{ /* Add new patch number below this line */
+ /**/
+ 87,
/**/
--
The technology involved in making anything invisible is so infinitely
complex that nine hundred and ninety-nine billion, nine hundred and
ninety-nine million, nine hundred and ninety-nine thousand, nine hundred
and ninety-nine times out of a trillion it is much simpler and more
effective just to take the thing away and do without it.
-- Douglas Adams, "The Hitchhiker's Guide to the Galaxy"
/// Bram Moolenaar -- [email protected] -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
