bug#11273: 24.0.94; quitting gdb

bug#11273: 24.0.94; quitting gdb

[Sam Steingold]

I start gdb on an interactive process (clisp) which I then exit.
Now, I quit gdb too, but I am still left with a bunch of gdb windows and
emacs running at 100% cpu because
(process-status "gdb-inferior")
=> run
even though it actually does not run (according to ps).
(delete-process "gdb-inferior") fixes that, but this should not be
necessary.

1. BUG: even though the gdb-inferior process is not running, it is
reported as running and emacs takes 100% cpu.

2. RFE: command gud-quit which would
- delete all gud processes
- quit-window all gud windows


In GNU Emacs 24.0.94.5 (x86_64-unknown-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2012-03-29 on t520sds
Windowing system distributor `The X.Org Foundation', version 11.0.11004000
Configured using:
 `configure '--with-wide-int''

-- 
Sam Steingold (http://sds.podval.org/) on Ubuntu 11.10 (oneiric) X 11.0.11004000
http://www.childpsy.net/ http://openvotingconsortium.org
http://memri.org http://jihadwatch.org http://dhimmi.com http://ffii.org
What's the difference between Apathy & Ignorance? -I don't know and don't care!

bug#11273: 24.0.94; quitting gdb

[Chong Yidong]

Sam Steingold <sds@gnu.org> writes:

> I start gdb on an interactive process (clisp) which I then exit.
> Now, I quit gdb too, but I am still left with a bunch of gdb windows and
> emacs running at 100% cpu because
> (process-status "gdb-inferior")
> => run
> even though it actually does not run (according to ps).
> (delete-process "gdb-inferior") fixes that, but this should not be
> necessary.
>
> 1. BUG: even though the gdb-inferior process is not running, it is
> reported as running and emacs takes 100% cpu.

This seems to be a combination of two bugs in the process code.

The situation is as follows: gdb-mi creates a pty process in the
"gdb-inferior" buffer, and hooks that pty up to the main gdb process.

The first bug is that it's necessary to explicitly call delete-process
on pty processes.  The gdb-mi code did not explicitly do this.
Normally, when you kill a buffer Emacs offers to kill its processes, but
this is ineffective for ptys, as the kill_buffer_processes() code calls
kill-process, not delete-process.  You can show this problem with the
following code unrelated to gdb:

  M-: (start-process "foo" (current-buffer) nil) RET
  C-x k RET yes RET
  M-x list-processes RET

Even though the pty's buffer is killed, and the user was prompted
whether to kill the process and answered yes, the pty is still
"running".

I don't know whether we should fix this general bug for 24.1---Stefan,
any opinion?

In the meantime, I installed a workaround to explicitly call
delete-process on the pty in the sentinel of the main gdb process.


Here is the second bug.  Uunder some circumstances, when the gdb process
exits, the sentinel doesn't get run.  You can see this by instrumenting
the `gud-sentinel' function, starting `M-x gdb', issuing an "r" command,
then "q".  The sentinel is never called, even though list-processes
confirms that the gdb process is now dead.  I have no idea why this
happens, but according to a debugging session it appears that the
select() call in process.c:4457 always returns a value >=0, so that
status_notify() is never called.  Maybe this has to do with the presence
of the gdb-inferior pty?

bug#11273: 24.0.94; quitting gdb

[Glenn Morris]

Chong Yidong wrote:

> In the meantime, I installed a workaround to explicitly call
> delete-process on the pty in the sentinel of the main gdb process.

Maybe you fixed bug#4437 then?

bug#11273: 24.0.94; quitting gdb

[Chong Yidong]

Glenn Morris <rgm@gnu.org> writes:

> Chong Yidong wrote:
>
>> In the meantime, I installed a workaround to explicitly call
>> delete-process on the pty in the sentinel of the main gdb process.
>
> Maybe you fixed bug#4437 then?

Aha, looks like the situation is this:

In Bug#4437, Nick Roberts proposed a patch to process.c to avoid sending
a SIGCHLD to itself when it receives an EIO errno.  However, he noted
that this

   means that status_notify isn't called from
   wait_reading_process_output because this call is conditioned on
   select which returns a positive value (presumably because the pty's
   file descriptor hasn't been cleared).

which is the problem I noticed.  Nick didn't commit the patch at the
time.

In Bug#10933, Troels Nielsen independently proposed the same patch,
which was committed by Stefan on 2012-03-23.

The question is what to do about the problem.  I didn't test this, but
it may be that the patch causes ALL process sentinels to be blocked
while the gdb process is running, which is kinda unacceptable.

bug#11273: 24.0.94; quitting gdb

[Glenn Morris]

Also, I don't know if it has any relevance here, but just thought I'd
mention that I previously reverted some gdb-related process.c change
that was causing general problems - bug#9839.

bug#11273: 24.0.94; quitting gdb

[Troels Nielsen]

Chong Yidong writes:
 > Glenn Morris <rgm@gnu.org> writes:
 > 
 > > Chong Yidong wrote:
 > >
 > >> In the meantime, I installed a workaround to explicitly call
 > >> delete-process on the pty in the sentinel of the main gdb process.
 > >
 > > Maybe you fixed bug#4437 then?
 > 
 > Aha, looks like the situation is this:
 > 
 > In Bug#4437, Nick Roberts proposed a patch to process.c to avoid sending
 > a SIGCHLD to itself when it receives an EIO errno.  However, he noted
 > that this
 > 
 >    means that status_notify isn't called from
 >    wait_reading_process_output because this call is conditioned on
 >    select which returns a positive value (presumably because the pty's
 >    file descriptor hasn't been cleared).
 > 
 > which is the problem I noticed.  Nick didn't commit the patch at the
 > time.
 > 
 > In Bug#10933, Troels Nielsen independently proposed the same patch,
 > which was committed by Stefan on 2012-03-23.
 > 
 > The question is what to do about the problem.  I didn't test this, but
 > it may be that the patch causes ALL process sentinels to be blocked
 > while the gdb process is running, which is kinda unacceptable.

Hi Chong

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.

I will look into a different way of solving #10933, but I think for
now my changes should be removed.

Regards
Troels

bug#11273: 24.0.94; quitting gdb

[Chong Yidong]

Troels Nielsen <bn.troels@gmail.com> 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

bug#11273: 24.0.94; quitting gdb

[Chong Yidong]

Chong Yidong <cyd@gnu.org> writes:

> 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.

Committed to emacs-24 branch.