Subject: Re: bug#11273: 24.0.94; quitting gdb

From: Chong Yidong <[email-masked]>
Date: Fri, 20 Apr 2012 10:50:52 +0800

Troels Nielsen <[email-masked]> writes:

> I've looked into the issue more deeply and I think my suggested patch
> is most certainly wrong. The PTY somehow continously will tell select
> it has data available, but when emacs_read tries to read it nothing
> but an error and errno=IO is returned.

Yes, apparently we have no choice but to stop reading from the pty on
receiving EIO, otherwise Emacs will spin trying to read from it (that's
probably the reason for the gdb-mi causing 100% CPU someone reported).

I propose the following patch.  On getting EIO, Emacs sets up the pty
process object so that it runs its sentinel, with the `failed' process
status.  Then gdb-mi applies a sentinel to the inferior-io pty, and that
sentinel is responsible for allocating a new pty and hooking it to gdb.

If there are no objections, I will commit this shortly.

=== modified file 'lisp/progmodes/gdb-mi.el'
*** lisp/progmodes/gdb-mi.el	2012-04-19 08:09:30 +0000
--- lisp/progmodes/gdb-mi.el	2012-04-20 02:48:54 +0000
***************
*** 817,827 ****
              nil 'local)
    (local-set-key "\C-i" 'completion-at-point)
  
-   ;; FIXME: Under some circumstances, `gud-sentinel' apparently does
-   ;; not get called when the gdb process is killed (Bug#11273).
-   (add-hook 'post-command-hook 'gdb-inferior-io--maybe-delete-pty
- 	    nil t)
- 
    (setq gdb-first-prompt t)
    (setq gud-running nil)
  
--- 817,822 ----
***************
*** 863,877 ****
  
    (gdb-get-buffer-create 'gdb-inferior-io)
    (gdb-clear-inferior-io)
!   (set-process-filter (get-process "gdb-inferior") 'gdb-inferior-filter)
!   (gdb-input
!    ;; Needs GDB 6.4 onwards
!    (concat "-inferior-tty-set "
! 	   (or
! 	    ;; The process can run on a remote host.
! 	    (process-get (get-process "gdb-inferior") 'remote-tty)
! 	    (process-tty-name (get-process "gdb-inferior"))))
!    'ignore)
    (if (eq window-system 'w32)
        (gdb-input "-gdb-set new-console off" 'ignore))
    (gdb-input "-gdb-set height 0" 'ignore)
--- 858,865 ----
  
    (gdb-get-buffer-create 'gdb-inferior-io)
    (gdb-clear-inferior-io)
!   (gdb-inferior-io--init-proc (get-process "gdb-inferior"))
! 
    (if (eq window-system 'w32)
        (gdb-input "-gdb-set new-console off" 'ignore))
    (gdb-input "-gdb-set height 0" 'ignore)
***************
*** 1522,1527 ****
--- 1510,1537 ----
  	 inf-pty
  	 (delete-process inf-pty))))
  
+ (defun gdb-inferior-io--init-proc (proc)
+   ;; Set up inferior I/O.  Needs GDB 6.4 onwards.
+   (set-process-filter proc 'gdb-inferior-filter)
+   (set-process-sentinel proc 'gdb-inferior-io-sentinel)
+   (gdb-input
+    (concat "-inferior-tty-set "
+ 	   ;; The process can run on a remote host.
+ 	   (or (process-get proc 'remote-tty)
+ 	       (process-tty-name proc)))
+    'ignore))
+ 
+ (defun gdb-inferior-io-sentinel (proc str)
+   (when (eq (process-status proc) 'failed)
+     ;; When the debugged process exits, Emacs gets an EIO on read from
+     ;; the pty.  We must remove the pty now (otherwise select() keeps
+     ;; triggering on this pty due to the EIO, and Emacs loops trying
+     ;; to read from it), and set up a new one.
+     (let ((buffer (process-buffer proc)))
+       ;; `comint-exec' deletes the original process as a side effect.
+       (comint-exec buffer "gdb-inferior" nil nil nil)
+       (gdb-inferior-io--init-proc (get-buffer-process buffer)))))
+ 
  (defconst gdb-frame-parameters
    '((height . 14) (width . 80)
      (unsplittable . t)

=== modified file 'src/process.c'
*** src/process.c	2012-04-18 07:21:18 +0000
--- src/process.c	2012-04-20 02:30:22 +0000
***************
*** 4893,4908 ****
  		 It can't hurt.  */
  	      else if (nread == -1 && errno == EIO)
  		{
!                   /* Don't do anything if only a pty, with no associated
! 		     process (bug#10933).  */
!                   if (XPROCESS (proc)->pid != -2) {
!                     /* Clear the descriptor now, so we only raise the signal
! 		       once.  */
!                     FD_CLR (channel, &input_wait_mask);
!                     FD_CLR (channel, &non_keyboard_wait_mask);
!                     
!                     kill (getpid (), SIGCHLD);
!                   }
  		}
  #endif /* HAVE_PTYS */
  	      /* If we can detect process termination, don't consider the
--- 4893,4915 ----
  		 It can't hurt.  */
  	      else if (nread == -1 && errno == EIO)
  		{
! 		  struct Lisp_Process *p = XPROCESS (proc);
! 
! 		  /* Clear the descriptor now, so we only raise the
! 		     signal once.  */
! 		  FD_CLR (channel, &input_wait_mask);
! 		  FD_CLR (channel, &non_keyboard_wait_mask);
! 
! 		  if (p->pid == -2)
! 		    {
! 		      /* If the EIO occurs on a pty, sigchld_handler's
! 			 wait3 will not find the process object to
! 			 mark for deletion.  Do that here.  */
! 		      p->tick = ++process_tick;
! 		      p->status = Qfailed;
! 		    }
!                   else
! 		    kill (getpid (), SIGCHLD);
  		}
  #endif /* HAVE_PTYS */
  	      /* If we can detect process termination, don't consider the