Patch 8.1.0845
Problem: Having job_status() free the job causes problems.
Solution: Do not actually free the job or terminal yet, put it in a list and
free it a bit later. Do not use a terminal after checking the job
status. (closes #3873)
Files: src/channel.c, src/terminal.c, src/proto/terminal.pro, src/misc2.c
*** ../vim-8.1.0844/src/channel.c 2019-01-28 20:04:20.324937137 +0100
--- src/channel.c 2019-01-29 22:08:49.222535249 +0100
***************
*** 5161,5168 ****
}
}
static void
! job_free_job(job_T *job)
{
if (job->jv_next != NULL)
job->jv_next->jv_prev = job->jv_prev;
--- 5161,5171 ----
}
}
+ /*
+ * Remove "job" from the list of jobs.
+ */
static void
! job_unlink(job_T *job)
{
if (job->jv_next != NULL)
job->jv_next->jv_prev = job->jv_prev;
***************
*** 5170,5175 ****
--- 5173,5184 ----
first_job = job->jv_next;
else
job->jv_prev->jv_next = job->jv_next;
+ }
+
+ static void
+ job_free_job(job_T *job)
+ {
+ job_unlink(job);
vim_free(job);
}
***************
*** 5183,5194 ****
--- 5192,5235 ----
}
}
+ job_T *jobs_to_free = NULL;
+
+ /*
+ * Put "job" in a list to be freed later, when it's no longer referenced.
+ */
+ static void
+ job_free_later(job_T *job)
+ {
+ job_unlink(job);
+ job->jv_next = jobs_to_free;
+ jobs_to_free = job;
+ }
+
+ static void
+ free_jobs_to_free_later(void)
+ {
+ job_T *job;
+
+ while (jobs_to_free != NULL)
+ {
+ job = jobs_to_free;
+ jobs_to_free = job->jv_next;
+ job_free_contents(job);
+ vim_free(job);
+ }
+ }
+
#if defined(EXITFREE) || defined(PROTO)
void
job_free_all(void)
{
while (first_job != NULL)
job_free(first_job);
+ free_jobs_to_free_later();
+
+ # ifdef FEAT_TERMINAL
+ free_unused_terminals();
+ # endif
}
#endif
***************
*** 5359,5364 ****
--- 5400,5407 ----
* 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).
+ * If the job is no longer used it will be removed from the list of jobs, and
+ * deleted a bit later.
*/
void
job_cleanup(job_T *job)
***************
*** 5394,5408 ****
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);
! }
}
/*
--- 5437,5449 ----
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. However, a caller might
! // still use it, thus free it a bit later.
! job_free_later(job);
}
/*
***************
*** 5609,5617 ****
if (job == NULL)
break;
did_end = TRUE;
! job_cleanup(job); // may free "job"
}
if (channel_need_redraw)
{
channel_need_redraw = FALSE;
--- 5650,5661 ----
if (job == NULL)
break;
did_end = TRUE;
! job_cleanup(job); // may add "job" to jobs_to_free
}
+ // Actually free jobs that were cleaned up.
+ free_jobs_to_free_later();
+
if (channel_need_redraw)
{
channel_need_redraw = FALSE;
*** ../vim-8.1.0844/src/terminal.c 2019-01-26 15:12:52.558260916 +0100
--- src/terminal.c 2019-01-29 22:27:59.339243929 +0100
***************
*** 803,812 ****
--- 803,819 ----
ga_clear(&term->tl_scrollback);
}
+
+ // Terminals that need to be freed soon.
+ term_T *terminals_to_free = NULL;
+
/*
* Free a terminal and everything it refers to.
* Kills the job if there is one.
* Called when wiping out a buffer.
+ * The actual terminal structure is freed later in free_unused_terminals(),
+ * because callbacks may wipe out a buffer while the terminal is still
+ * referenced.
*/
void
free_terminal(buf_T *buf)
***************
*** 816,821 ****
--- 823,830 ----
if (term == NULL)
return;
+
+ // Unlink the terminal form the list of terminals.
if (first_term == term)
first_term = term->tl_next;
else
***************
*** 834,860 ****
job_stop(term->tl_job, NULL, "kill");
job_unref(term->tl_job);
}
! free_scrollback(term);
! term_free_vterm(term);
! vim_free(term->tl_title);
#ifdef FEAT_SESSION
! vim_free(term->tl_command);
#endif
! vim_free(term->tl_kill);
! vim_free(term->tl_status_text);
! vim_free(term->tl_opencmd);
! vim_free(term->tl_eof_chars);
#ifdef WIN3264
! if (term->tl_out_fd != NULL)
! fclose(term->tl_out_fd);
#endif
! vim_free(term->tl_cursor_color);
! vim_free(term);
! buf->b_term = NULL;
! if (in_terminal_loop == term)
! in_terminal_loop = NULL;
}
/*
--- 843,883 ----
job_stop(term->tl_job, NULL, "kill");
job_unref(term->tl_job);
}
+ term->tl_next = terminals_to_free;
+ terminals_to_free = term;
! buf->b_term = NULL;
! if (in_terminal_loop == term)
! in_terminal_loop = NULL;
! }
! void
! free_unused_terminals()
! {
! while (terminals_to_free != NULL)
! {
! term_T *term = terminals_to_free;
!
! terminals_to_free = term->tl_next;
!
! free_scrollback(term);
!
! term_free_vterm(term);
! vim_free(term->tl_title);
#ifdef FEAT_SESSION
! vim_free(term->tl_command);
#endif
! vim_free(term->tl_kill);
! vim_free(term->tl_status_text);
! vim_free(term->tl_opencmd);
! vim_free(term->tl_eof_chars);
#ifdef WIN3264
! if (term->tl_out_fd != NULL)
! fclose(term->tl_out_fd);
#endif
! vim_free(term->tl_cursor_color);
! vim_free(term);
! }
}
/*
***************
*** 1275,1280 ****
--- 1298,1304 ----
/*
* Return TRUE if the job for "term" is still running.
* If "check_job_status" is TRUE update the job status.
+ * NOTE: "term" may be freed by callbacks.
*/
static int
term_job_running_check(term_T *term, int check_job_status)
***************
*** 1285,1294 ****
&& term->tl_job != NULL
&& channel_is_open(term->tl_job->jv_channel))
{
if (check_job_status)
! job_status(term->tl_job);
! return (term->tl_job->jv_status == JOB_STARTED
! || term->tl_job->jv_channel->ch_keep_open);
}
return FALSE;
}
--- 1309,1323 ----
&& term->tl_job != NULL
&& channel_is_open(term->tl_job->jv_channel))
{
+ job_T *job = term->tl_job;
+
+ // Careful: Checking the job status may invoked callbacks, which close
+ // the buffer and terminate "term". However, "job" will not be freed
+ // yet.
if (check_job_status)
! job_status(job);
! return (job->jv_status == JOB_STARTED
! || (job->jv_channel != NULL && job->jv_channel->ch_keep_open));
}
return FALSE;
}
***************
*** 2151,2159 ****
#ifdef FEAT_GUI
if (!curbuf->b_term->tl_system)
#endif
! /* TODO: skip screen update when handling a sequence of keys. */
! /* Repeat redrawing in case a message is received while redrawing.
! */
while (must_redraw != 0)
if (update_screen(0) == FAIL)
break;
--- 2180,2187 ----
#ifdef FEAT_GUI
if (!curbuf->b_term->tl_system)
#endif
! // TODO: skip screen update when handling a sequence of keys.
! // Repeat redrawing in case a message is received while redrawing.
while (must_redraw != 0)
if (update_screen(0) == FAIL)
break;
*** ../vim-8.1.0844/src/proto/terminal.pro 2019-01-20 15:30:36.893328693
+0100
--- src/proto/terminal.pro 2019-01-29 22:07:47.982845114 +0100
***************
*** 5,10 ****
--- 5,11 ----
int term_write_session(FILE *fd, win_T *wp);
int term_should_restore(buf_T *buf);
void free_terminal(buf_T *buf);
+ void free_unused_terminals(void);
void write_to_term(buf_T *buffer, char_u *msg, channel_T *channel);
int term_job_running(term_T *term);
int term_none_open(term_T *term);
*** ../vim-8.1.0844/src/misc2.c 2019-01-29 20:17:25.550548240 +0100
--- src/misc2.c 2019-01-29 22:07:18.670987912 +0100
***************
*** 6387,6392 ****
--- 6387,6395 ----
if (job_check_ended())
continue;
# endif
+ # ifdef FEAT_TERMINAL
+ free_unused_terminals();
+ # endif
break;
}
*** ../vim-8.1.0844/src/version.c 2019-01-29 20:36:53.350466201 +0100
--- src/version.c 2019-01-29 21:39:08.114348374 +0100
***************
*** 785,786 ****
--- 785,788 ----
{ /* Add new patch number below this line */
+ /**/
+ 845,
/**/
--
"Hegel was right when he said that we learn from history that man can
never learn anything from history." (George Bernard Shaw)
/// 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.
