patch 8.0.1795: lose contact with jobs when :gui forks

Problem:    Lose contact with jobs when :gui forks.
Solution:   Don't fork when there is a running job. Make log message for a
            died job clearer.  Also close the terminal when stderr and stdout
            are the same FD.

src/channel.c

@@ -485,7 +485,7 @@ channel_read_fd(int fd)
  */
 #ifdef FEAT_GUI_X11
     static void
-messageFromServer(XtPointer clientData,
+messageFromServerX11(XtPointer clientData,
 		  int *unused1 UNUSED,
 		  XtInputId *unused2 UNUSED)
 {
@@ -496,7 +496,7 @@ messageFromServer(XtPointer clientData,
 #ifdef FEAT_GUI_GTK
 # if GTK_CHECK_VERSION(3,0,0)
     static gboolean
-messageFromServer(GIOChannel *unused1 UNUSED,
+messageFromServerGtk3(GIOChannel *unused1 UNUSED,
 		  GIOCondition unused2 UNUSED,
 		  gpointer clientData)
 {
@@ -506,7 +506,7 @@ messageFromServer(GIOChannel *unused1 UNUSED,
 }
 # else
     static void
-messageFromServer(gpointer clientData,
+messageFromServerGtk2(gpointer clientData,
 		  gint unused1 UNUSED,
 		  GdkInputCondition unused2 UNUSED)
 {
@@ -526,41 +526,48 @@ channel_gui_register_one(channel_T *channel, ch_part_T part)
 	return;
 
 # ifdef FEAT_GUI_X11
-    /* Tell notifier we are interested in being called
+    /* Tell notifier we are interested in being called when there is input on
-     * when there is input on the editor connection socket. */
+     * the editor connection socket. */
     if (channel->ch_part[part].ch_inputHandler == (XtInputId)NULL)
+    {
+	ch_log(channel, "Registering part %s with fd %d",
+		part_names[part], channel->ch_part[part].ch_fd);
+
 	channel->ch_part[part].ch_inputHandler = XtAppAddInput(
 		(XtAppContext)app_context,
 		channel->ch_part[part].ch_fd,
 		(XtPointer)(XtInputReadMask + XtInputExceptMask),
-		messageFromServer,
+		messageFromServerX11,
 		(XtPointer)(long)channel->ch_part[part].ch_fd);
+    }
 # else
 #  ifdef FEAT_GUI_GTK
-    /* Tell gdk we are interested in being called when there
+    /* Tell gdk we are interested in being called when there is input on the
-     * is input on the editor connection socket. */
+     * editor connection socket. */
     if (channel->ch_part[part].ch_inputHandler == 0)
-#   if GTK_CHECK_VERSION(3,0,0)
     {
+	ch_log(channel, "Registering part %s with fd %d",
+		part_names[part], channel->ch_part[part].ch_fd);
+#   if GTK_CHECK_VERSION(3,0,0)
 	GIOChannel *chnnl = g_io_channel_unix_new(
 		(gint)channel->ch_part[part].ch_fd);
 
 	channel->ch_part[part].ch_inputHandler = g_io_add_watch(
 		chnnl,
 		G_IO_IN|G_IO_HUP|G_IO_ERR|G_IO_PRI,
-		messageFromServer,
+		messageFromServerGtk3,
 		GINT_TO_POINTER(channel->ch_part[part].ch_fd));
 
 	g_io_channel_unref(chnnl);
-    }
 #   else
 	channel->ch_part[part].ch_inputHandler = gdk_input_add(
 		(gint)channel->ch_part[part].ch_fd,
 		(GdkInputCondition)
 			     ((int)GDK_INPUT_READ + (int)GDK_INPUT_EXCEPTION),
-		messageFromServer,
+		messageFromServerGtk2,
 		(gpointer)(long)channel->ch_part[part].ch_fd);
 #   endif
+    }
 #  endif
 # endif
 }
@@ -598,6 +605,7 @@ channel_gui_unregister_one(channel_T *channel, ch_part_T part)
 # ifdef FEAT_GUI_X11
     if (channel->ch_part[part].ch_inputHandler != (XtInputId)NULL)
     {
+	ch_log(channel, "Unregistering part %s", part_names[part]);
 	XtRemoveInput(channel->ch_part[part].ch_inputHandler);
 	channel->ch_part[part].ch_inputHandler = (XtInputId)NULL;
     }
@@ -605,6 +613,7 @@ channel_gui_unregister_one(channel_T *channel, ch_part_T part)
 #  ifdef FEAT_GUI_GTK
     if (channel->ch_part[part].ch_inputHandler != 0)
     {
+	ch_log(channel, "Unregistering part %s", part_names[part]);
 #   if GTK_CHECK_VERSION(3,0,0)
 	g_source_remove(channel->ch_part[part].ch_inputHandler);
 #   else
@@ -3245,7 +3254,16 @@ ch_close_part_on_error(
 			      (int)STRLEN(DETACH_MSG_RAW), FALSE, "PUT ");
 
     /* When reading is not possible close this part of the channel.  Don't
-     * close the channel yet, there may be something to read on another part. */
+     * close the channel yet, there may be something to read on another part.
+     * When stdout and stderr use the same FD we get the error only on one of
+     * them, also close the other. */
+    if (part == PART_OUT || part == PART_ERR)
+    {
+	ch_part_T other = part == PART_OUT ? PART_ERR : PART_OUT;
+
+	if (channel->ch_part[part].ch_fd == channel->ch_part[other].ch_fd)
+	    ch_close_part(channel, other);
+    }
     ch_close_part(channel, part);
 
 #ifdef FEAT_GUI
@@ -5115,6 +5133,22 @@ job_still_useful(job_T *job)
     return job_need_end_check(job) || job_channel_still_useful(job);
 }
 
+#if defined(GUI_MAY_FORK) || defined(PROTO)
+/*
+ * Return TRUE when there is any running job that we care about.
+ */
+    int
+job_any_running()
+{
+    job_T	*job;
+
+    for (job = first_job; job != NULL; job = job->jv_next)
+	if (job_still_useful(job))
+	    return TRUE;
+    return FALSE;
+}
+#endif
+
 #if !defined(USE_ARGV) || defined(PROTO)
 /*
  * Escape one argument for an external command.

src/gui.c

@@ -37,8 +37,7 @@ static void gui_set_fg_color(char_u *name);
 static void gui_set_bg_color(char_u *name);
 static win_T *xy2win(int x, int y);
 
-#if defined(UNIX) && !defined(FEAT_GUI_MAC)
+#ifdef GUI_MAY_FORK
-# define MAY_FORK
 static void gui_do_fork(void);
 
 static int gui_read_child_pipe(int fd);
@@ -49,8 +48,7 @@ enum {
     GUI_CHILD_OK,
     GUI_CHILD_FAILED
 };
-
+#endif
-#endif /* MAY_FORK */
 
 static void gui_attempt_start(void);
 
@@ -88,14 +86,20 @@ gui_start(void)
 
     ++recursive;
 
-#ifdef MAY_FORK
+#ifdef GUI_MAY_FORK
     /*
      * Quit the current process and continue in the child.
      * Makes "gvim file" disconnect from the shell it was started in.
      * Don't do this when Vim was started with "-f" or the 'f' flag is present
      * in 'guioptions'.
+     * Don't do this when there is a running job, we can only get the status
+     * of a child from the parent.
      */
-    if (gui.dofork && !vim_strchr(p_go, GO_FORG) && recursive <= 1)
+    if (gui.dofork && !vim_strchr(p_go, GO_FORG) && recursive <= 1
+# ifdef FEAT_JOB_CHANNEL
+	    && !job_any_running()
+# endif
+	    )
     {
 	gui_do_fork();
     }
@@ -183,7 +187,7 @@ gui_attempt_start(void)
     --recursive;
 }
 
-#ifdef MAY_FORK
+#ifdef GUI_MAY_FORK
 
 /* for waitpid() */
 # if defined(HAVE_SYS_WAIT_H) || defined(HAVE_UNION_WAIT)
@@ -338,7 +342,7 @@ gui_read_child_pipe(int fd)
     return GUI_CHILD_FAILED;
 }
 
-#endif /* MAY_FORK */
+#endif /* GUI_MAY_FORK */
 
 /*
  * Call this when vim starts up, whether or not the GUI is started

src/gui.h

@@ -564,3 +564,7 @@ typedef enum
 #  define FUNC2GENERIC(func) G_CALLBACK(func)
 # endif
 #endif /* FEAT_GUI_GTK */
+
+#if defined(UNIX) && !defined(FEAT_GUI_MAC)
+# define GUI_MAY_FORK
+#endif

src/os_unix.c

@@ -5642,8 +5642,10 @@ mch_job_start(char **argv, job_T *job, jobopt_T *options)
 			? INVALID_FD : fd_in[1] < 0 ? pty_master_fd : fd_in[1];
 	int out_fd = use_file_for_out || use_null_for_out
 		      ? INVALID_FD : fd_out[0] < 0 ? pty_master_fd : fd_out[0];
+	/* When using pty_master_fd only set it for stdout, do not duplicate it
+	 * for stderr, it only needs to be read once. */
 	int err_fd = use_out_for_err || use_file_for_err || use_null_for_err
-		      ? INVALID_FD : fd_err[0] < 0 ? pty_master_fd : fd_err[0];
+		      ? INVALID_FD : fd_err[0] < 0 ? INVALID_FD : fd_err[0];
 
 	channel_set_pipes(channel, in_fd, out_fd, err_fd);
 	channel_set_job(channel, job, options);
@@ -5701,6 +5703,9 @@ mch_job_status(job_T *job)
     if (wait_pid == -1)
     {
 	/* process must have exited */
+	if (job->jv_status < JOB_ENDED)
+	    ch_log(job->jv_channel, "Job no longer exists: %s",
+							      strerror(errno));
 	goto return_dead;
     }
     if (wait_pid == 0)
@@ -5709,21 +5714,22 @@ mch_job_status(job_T *job)
     {
 	/* LINTED avoid "bitwise operation on signed value" */
 	job->jv_exitval = WEXITSTATUS(status);
+	if (job->jv_status < JOB_ENDED)
+	    ch_log(job->jv_channel, "Job exited with %d", job->jv_exitval);
 	goto return_dead;
     }
     if (WIFSIGNALED(status))
     {
 	job->jv_exitval = -1;
+	if (job->jv_status < JOB_ENDED)
+	    ch_log(job->jv_channel, "Job terminated by a signal");
 	goto return_dead;
     }
     return "run";
 
 return_dead:
     if (job->jv_status < JOB_ENDED)
-    {
-	ch_log(job->jv_channel, "Job ended");
 	job->jv_status = JOB_ENDED;
-    }
     return "dead";
 }
 
@@ -5857,7 +5863,9 @@ mch_create_pty_channel(job_T *job, jobopt_T *options)
     job->jv_channel = channel;  /* ch_refcount was set by add_channel() */
     channel->ch_keep_open = TRUE;
 
-    channel_set_pipes(channel, pty_master_fd, pty_master_fd, pty_master_fd);
+    /* Only set the pty_master_fd for stdout, do not duplicate it for stderr,
+     * it only needs to be read once. */
+    channel_set_pipes(channel, pty_master_fd, pty_master_fd, INVALID_FD);
     channel_set_job(channel, job, options);
     return OK;
 }

src/proto/channel.pro

@@ -54,6 +54,7 @@ void free_job_options(jobopt_T *opt);
 int get_job_options(typval_T *tv, jobopt_T *opt, int supported, int supported2);
 channel_T *get_channel_arg(typval_T *tv, int check_open, int reading, ch_part_T part);
 void job_free_all(void);
+int job_any_running(void);
 int win32_build_cmd(list_T *l, garray_T *gap);
 void job_cleanup(job_T *job);
 int set_ref_in_job(int copyID);

src/terminal.c

@@ -42,9 +42,6 @@
  *   redirection.  Probably in call to channel_set_pipes().
  * - Win32: Redirecting output does not work, Test_terminal_redir_file()
  *   is disabled.
- * - When starting terminal window with shell in terminal, then using :gui to
- *   switch to GUI, shell stops working. Scrollback seems wrong, command
- *   running in shell is still running.
  * - GUI: when using tabs, focus in terminal, click on tab does not work.
  * - handle_moverect() scrolls one line at a time.  Postpone scrolling, count
  *   the number of lines, until a redraw happens.  Then if scrolling many lines

src/version.c

@@ -761,6 +761,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1795,
 /**/
     1794,
 /**/