diff src/terminal.c @ 15675:01890a3caefd v8.1.0845

patch 8.1.0845: having job_status() free the job causes problems commit https://github.com/vim/vim/commit/2a4857a1fcf1d188e5b985ac21bcfc532eddde94 Author: Bram Moolenaar <Bram@vim.org> Date: Tue Jan 29 22:29:07 2019 +0100 patch 8.1.0845: having job_status() free the job causes problems 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)
author Bram Moolenaar <Bram@vim.org>
date Tue, 29 Jan 2019 22:30:06 +0100
parents d4a6d575e910
children d8b4cbb14e1e
line wrap: on
line diff
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -803,10 +803,17 @@ free_scrollback(term_T *term)
     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,6 +823,8 @@ free_terminal(buf_T *buf)
 
     if (term == NULL)
 	return;
+
+    // Unlink the terminal form the list of terminals.
     if (first_term == term)
 	first_term = term->tl_next;
     else
@@ -834,29 +843,43 @@ free_terminal(buf_T *buf)
 	    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);
+    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);
+    }
+}
+
 /*
  * Get the part that is connected to the tty. Normally this is PART_IN, but
  * when writing buffer lines to the job it can be another.  This makes it
@@ -1275,6 +1298,7 @@ term_convert_key(term_T *term, int c, ch
 /*
  * 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,10 +1309,15 @@ term_job_running_check(term_T *term, int
 	&& 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(term->tl_job);
-	return (term->tl_job->jv_status == JOB_STARTED
-		|| term->tl_job->jv_channel->ch_keep_open);
+	    job_status(job);
+	return (job->jv_status == JOB_STARTED
+		|| (job->jv_channel != NULL && job->jv_channel->ch_keep_open));
     }
     return FALSE;
 }
@@ -2151,9 +2180,8 @@ terminal_loop(int blocking)
 #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.
-	     */
+	    // 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;