To: vim_dev@googlegroups.com Subject: Patch 8.0.1795 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.0.1795 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. Files: src/gui.h, src/gui.c, src/channel.c, src/proto/channel.pro, src/os_unix.c, src/terminal.c *** ../vim-8.0.1794/src/gui.h 2018-02-27 14:38:58.653004777 +0100 --- src/gui.h 2018-05-05 18:42:37.345820969 +0200 *************** *** 564,566 **** --- 564,570 ---- # define FUNC2GENERIC(func) G_CALLBACK(func) # endif #endif /* FEAT_GUI_GTK */ + + #if defined(UNIX) && !defined(FEAT_GUI_MAC) + # define GUI_MAY_FORK + #endif *** ../vim-8.0.1794/src/gui.c 2018-03-04 21:35:54.952059810 +0100 --- src/gui.c 2018-05-05 18:46:56.728397368 +0200 *************** *** 37,44 **** static void gui_set_bg_color(char_u *name); static win_T *xy2win(int x, int y); ! #if defined(UNIX) && !defined(FEAT_GUI_MAC) ! # define MAY_FORK static void gui_do_fork(void); static int gui_read_child_pipe(int fd); --- 37,43 ---- static void gui_set_bg_color(char_u *name); static win_T *xy2win(int x, int y); ! #ifdef GUI_MAY_FORK static void gui_do_fork(void); static int gui_read_child_pipe(int fd); *************** *** 49,56 **** GUI_CHILD_OK, GUI_CHILD_FAILED }; ! ! #endif /* MAY_FORK */ static void gui_attempt_start(void); --- 48,54 ---- GUI_CHILD_OK, GUI_CHILD_FAILED }; ! #endif static void gui_attempt_start(void); *************** *** 88,101 **** ++recursive; ! #ifdef 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'. */ ! if (gui.dofork && !vim_strchr(p_go, GO_FORG) && recursive <= 1) { gui_do_fork(); } --- 86,105 ---- ++recursive; ! #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 ! # ifdef FEAT_JOB_CHANNEL ! && !job_any_running() ! # endif ! ) { gui_do_fork(); } *************** *** 183,189 **** --recursive; } ! #ifdef MAY_FORK /* for waitpid() */ # if defined(HAVE_SYS_WAIT_H) || defined(HAVE_UNION_WAIT) --- 187,193 ---- --recursive; } ! #ifdef GUI_MAY_FORK /* for waitpid() */ # if defined(HAVE_SYS_WAIT_H) || defined(HAVE_UNION_WAIT) *************** *** 338,344 **** return GUI_CHILD_FAILED; } ! #endif /* MAY_FORK */ /* * Call this when vim starts up, whether or not the GUI is started --- 342,348 ---- return GUI_CHILD_FAILED; } ! #endif /* GUI_MAY_FORK */ /* * Call this when vim starts up, whether or not the GUI is started *** ../vim-8.0.1794/src/channel.c 2018-04-30 10:38:35.772177610 +0200 --- src/channel.c 2018-05-05 20:55:02.991113683 +0200 *************** *** 485,491 **** */ #ifdef FEAT_GUI_X11 static void ! messageFromServer(XtPointer clientData, int *unused1 UNUSED, XtInputId *unused2 UNUSED) { --- 485,491 ---- */ #ifdef FEAT_GUI_X11 static void ! messageFromServerX11(XtPointer clientData, int *unused1 UNUSED, XtInputId *unused2 UNUSED) { *************** *** 496,502 **** #ifdef FEAT_GUI_GTK # if GTK_CHECK_VERSION(3,0,0) static gboolean ! messageFromServer(GIOChannel *unused1 UNUSED, GIOCondition unused2 UNUSED, gpointer clientData) { --- 496,502 ---- #ifdef FEAT_GUI_GTK # if GTK_CHECK_VERSION(3,0,0) static gboolean ! messageFromServerGtk3(GIOChannel *unused1 UNUSED, GIOCondition unused2 UNUSED, gpointer clientData) { *************** *** 506,512 **** } # else static void ! messageFromServer(gpointer clientData, gint unused1 UNUSED, GdkInputCondition unused2 UNUSED) { --- 506,512 ---- } # else static void ! messageFromServerGtk2(gpointer clientData, gint unused1 UNUSED, GdkInputCondition unused2 UNUSED) { *************** *** 526,566 **** return; # ifdef FEAT_GUI_X11 ! /* Tell notifier we are interested in being called ! * when there is input on the editor connection socket. */ if (channel->ch_part[part].ch_inputHandler == (XtInputId)NULL) channel->ch_part[part].ch_inputHandler = XtAppAddInput( (XtAppContext)app_context, channel->ch_part[part].ch_fd, (XtPointer)(XtInputReadMask + XtInputExceptMask), ! messageFromServer, (XtPointer)(long)channel->ch_part[part].ch_fd); # else # ifdef FEAT_GUI_GTK ! /* Tell gdk we are interested in being called when there ! * is input on the editor connection socket. */ if (channel->ch_part[part].ch_inputHandler == 0) - # 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, 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, (gpointer)(long)channel->ch_part[part].ch_fd); # endif # endif # endif } --- 526,573 ---- return; # ifdef FEAT_GUI_X11 ! /* Tell notifier we are interested in being called when there is input on ! * 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), ! messageFromServerX11, (XtPointer)(long)channel->ch_part[part].ch_fd); + } # else # ifdef FEAT_GUI_GTK ! /* Tell gdk we are interested in being called when there is input on the ! * editor connection socket. */ if (channel->ch_part[part].ch_inputHandler == 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, ! 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), ! messageFromServerGtk2, (gpointer)(long)channel->ch_part[part].ch_fd); # endif + } # endif # endif } *************** *** 598,603 **** --- 605,611 ---- # 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,610 **** --- 613,619 ---- # 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,3251 **** (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. */ ch_close_part(channel, part); #ifdef FEAT_GUI --- 3254,3269 ---- (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. ! * 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,5120 **** --- 5133,5154 ---- 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. *** ../vim-8.0.1794/src/proto/channel.pro 2018-04-21 19:49:02.528104537 +0200 --- src/proto/channel.pro 2018-05-05 18:48:46.927873840 +0200 *************** *** 54,59 **** --- 54,60 ---- 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); *** ../vim-8.0.1794/src/os_unix.c 2018-04-24 20:54:01.572445034 +0200 --- src/os_unix.c 2018-05-05 20:58:53.541489797 +0200 *************** *** 5642,5649 **** ? 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]; 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]; channel_set_pipes(channel, in_fd, out_fd, err_fd); channel_set_job(channel, job, options); --- 5642,5651 ---- ? 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 ? INVALID_FD : fd_err[0]; channel_set_pipes(channel, in_fd, out_fd, err_fd); channel_set_job(channel, job, options); *************** *** 5701,5706 **** --- 5703,5711 ---- 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,5729 **** { /* LINTED avoid "bitwise operation on signed value" */ job->jv_exitval = WEXITSTATUS(status); goto return_dead; } if (WIFSIGNALED(status)) { job->jv_exitval = -1; 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"; } --- 5714,5735 ---- { /* 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) job->jv_status = JOB_ENDED; return "dead"; } *************** *** 5857,5863 **** 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); channel_set_job(channel, job, options); return OK; } --- 5863,5871 ---- job->jv_channel = channel; /* ch_refcount was set by add_channel() */ channel->ch_keep_open = TRUE; ! /* 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; } *** ../vim-8.0.1794/src/terminal.c 2018-05-05 16:36:00.483925176 +0200 --- src/terminal.c 2018-05-05 19:21:26.800137872 +0200 *************** *** 42,50 **** * 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 --- 42,47 ---- *** ../vim-8.0.1794/src/version.c 2018-05-05 16:36:00.483925176 +0200 --- src/version.c 2018-05-05 19:43:12.679569602 +0200 *************** *** 763,764 **** --- 763,766 ---- { /* Add new patch number below this line */ + /**/ + 1795, /**/ -- ARTHUR: Be quiet! DENNIS: --but by a two-thirds majority in the case of more-- ARTHUR: Be quiet! I order you to be quiet! WOMAN: Order, eh -- who does he think he is? ARTHUR: I am your king! The Quest for the Holy Grail (Monty Python) /// Bram Moolenaar -- Bram@Moolenaar.net -- 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 ///