bug#13845: 24.3.50; gud commands stop responding when using gdb-mi

bug#13845: 24.3.50; gud commands stop responding when using gdb-mi

[Jean-Philippe Gravel]

When debugging a program using gdb-mi, the gud commands (gud-break for
instance) stop responding.  I get the problem all the time on the
trunk version of Emacs (revision 111899).  I have gdb version 7.5.1
installed.  The following steps should reproduce the problem:

Start Emacs:
  cd trunk/src
  ./emacs -Q

Start a another Emacs in gdb-mi.  Use the CLI interface to start the program:
  M-x gdb
  Run gdb (like this): gdb -i=mi emacs
  (gdb) run

Back in the first emacs, break GDB:
  C-c C-c

Open one of the Emacs source file (emacs.c)
  C-x C-f
  Find file: ../trunk/src/emacs.c

In the function main, position the cursor on a line of code and try to
set a break point using the gud command:
  M-x gud-break

The break point is successfully created.  Return to the *gud-emacs*
buffer and resume the debugged Emacs, then break again:
  (gdb) continue
  C-c C-c

Go back in the emacs.c file and try to set a breakpoint again in the
main function.  From this point on, the gud-break will refuse to work.
 The internal state of gdb-mi is in fact in a stall.  Several other
features of gdb-mi are also disabled.  For instance, if you start
gdb-many-windows:
  M-x gdb-many-windows

and click on the Threads tab, the buffer will be empty even though the
debugged program isn't running.

I investigated the problem and found it's root in the gdb-mi.el file.
When typing "run" in the debugger, GDB responds with a "^running"
message.  gdb-mi handles this by calling the gdb-starting function.
In this function, you will find the following:
  (defun gdb-starting (_output-field)
    [...]
    ;; GDB doesn't seem to respond to -thread-info before first stop or
    ;; thread exit (even in non-stop mode), so this is useless.
    ;; Behavior may change in the future.
    (gdb-emit-signal gdb-buf-publisher 'update-threads))

Indeed, GDB doesn't respond to the -thread-info command.
Nevertheless, gdb-mi records the -thread-info request in the variable
gdb-pending-trigger and will refuse to send any other -thread-info
command until GDB responds the command gdb-mi is waiting for.

Back to our stalled gdb-mi, if we probe the gdb-pending-trigger
variable, we get:
  C-h v
  Describe variable: gdb-pending-trigger
    gdb-pending-triggers is a variable defined in `gdb-mi.el'.
    Its value is ((#<buffer *threads of emacs*> . gdb-invalidate-threads))

We can get back on our feet by clearing that variable:
  M-:
  Eval: (setq gdb-pending-triggers nil)

and then run a CLI command in GDB:
  (gdb) where

Only resetting the variable is not enough on it's own, but running a
CLI command right after seems to wake-up gdb-mi out of it's coma.
After this, you can run gud commands again and gdb-many-windows is
back to life.

Commenting out the (gdb-emit-signal gdb-buf-publisher 'update-threads)
statement in both gdb-starting and gdb-running helps quite a bit, but
this fix is not enough.  I sometimes encounter cases where gdb-mi
fails to receive replies for a bunch of other commands to update all
other gdb window (breakpoints, call-stack, etc.)  Unfortunately, this
condition is harder to encounter and I cannot provide repro-steps at
this point.

I do not know if this should be considered an emacs or a GDB bug
(should GDB always reply to commands, even when the program is
running?)  What I can say for sure is that there is definitively room
for improvement on the emacs side: the pending message handling should
be more robust and gdb-mi should not hang if GDB skips a reply.


In GNU Emacs 24.3.50.1 (i686-pc-linux-gnu, GTK+ Version 3.4.2)
 of 2013-02-28 on ubuntu
Bzr revision: 111899 juri@jurta.org-20130228215111-mwv0v6velfkb8vbw
Windowing system distributor `The X.Org Foundation', version 11.0.11103000
System Description:	Ubuntu 12.04.1 LTS

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: C/l

Minor modes in effect:
  gdb-many-windows: t
  tooltip-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t

Recent input:
M-x g d b <return> <C-backspace> <C-backspace> e m
a c s <return> r u n <return> C-c C-c C-x C-f e m a
c s . c <return> C-s m a i n SPC ( C-s <down> <down>
<down> C-x SPC C-x b <return> c o n t i n u e <return>
C-c C-c C-x b <return> <down> <down> C-x SPC C-x b
<return> M-x g d b - m a n y - w i n d o w <return>
<help-echo> <help-echo> <down-mouse-1> <mouse-1> <help-echo>
<help-echo> <help-echo> C-h v g d b - p e n d i n g
<tab> <return> q M-: ( s e t q SPC g d b - p e n d
i n <tab> t r <tab> SPC n i l ) <return> w h e r e
<return> <help-echo> <help-echo> <help-echo> <help-echo>
<help-echo> <down-mouse-1> <mouse-1> C-x SPC <help-echo>
M-x r e p o r t - e m a c s - b u g <return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Switched to thread 1
Loading cc-langs...done
Mark saved where search started
Command: break emacs.c:654
Gdb-Many-Windows mode enabled
Type "q" to restore previous buffer.
nil
Command: break emacs.c:656

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message cl-macs gv format-spec
rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils pp help-mode help-fns cus-start cus-load
misearch multi-isearch vc-bzr cc-langs cl nadvice cl-lib cc-mode
cc-fonts easymenu cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine
cc-vars cc-defs gdb-mi bindat json gud easy-mmode comint ansi-color ring
time-date tooltip ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
lisp-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer loaddefs button faces cus-face macroexp files text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process inotify
dynamic-setting system-font-setting font-render-setting move-toolbar gtk
x-toolkit x multi-tty emacs)

bug#13845: 24.3.50; gud commands stop responding when using gdb-mi

[Jean-Philippe Gravel]

Hi,

I have a fix ready for this bug.  I just got write access to the
repository and this will be the first commit I do on my own. I would
therefore like to request a code review to make sure I got everything
right.

My branch is available on launchpad:
https://code.launchpad.net/~jpgravel/emacs/bug13845

Please consider all revisions from 112234 to 112242.  Revision 112233
is already in the trunk.

The core of this branch consists of a rework of the way pending
triggers are implemented.  The new implementation will allow easy
cleanup of dead pending triggers, whenever they are detected.  There
used to be two lists to keep track of the commands sent to gdb:
gdb-handler-alist and gdb-pending-triggers.  The first one was storing
the callback to invoke when receiving the reply form GDB, the second
was flagging some of those commands as pending, to prevent GDB from
sending the same commands until the reply is received.  With my
changes, the two lists are merged into gdb-handler-list, making it
easier to cleanly insert and remove handlers.

I thought of two ways of detecting dead pending triggers:
 1 - It seems that GDB always replies in the same order Emacs sends
commands.  Replies from GDB are therefore always received in
increasing numerical order.  Based on this observation, we can remove
old pending triggers as soon as newer replies are received from GDB.
 2 - We could add a timeout mechanism that would remove pending
triggers if no replies is received after a certain amount of time.

With this branch, I only implemented option 1, which can be
enabled/disabled using `gdb-discard-unordered-replies'.  I may
implement Option 2 in a subsequent commit.

Regarding this branch, I have three questions:
 - I needed the function cl-find-if and cl-delete-if.  It seems there
is a long history about not using the functions from cl.el at runtime.
 It also seems that lots of devs simply re-implemented those functions
in their own modules.  I couldn't find the functions I needed in a
genetic utility package (other than cl.el).  Instead, I found them
re-implemented as erc-delete-if and org-find-if.  For now, I copied
those two functions in gdb-mi.el but I can't help but wonder if there
is a place where those functions could be moved so that different
modules could share their implementation.
 - I removed two strange ^L characters in gdb-mi.el.  Am I right to
remove them?  Do they mean anything, or are they garbage ASCII
character that got there by mistake?
 - When doing my commit, do I need to specify a bug tracker with the
--fixes option, or is "--fixes 13845" enough?

Thanks,
Jean-Philippe

bug#13845: 24.3.50; gud commands stop responding when using gdb-mi

[Glenn Morris]

Jean-Philippe Gravel wrote:

> My branch is available on launchpad:
> https://code.launchpad.net/~jpgravel/emacs/bug13845
>
> Please consider all revisions from 112234 to 112242.  Revision 112233
> is already in the trunk.

Could you send the change you propose to install to this address as a
flat diff? I'm assuming it is not enormous.

>  - I needed the function cl-find-if and cl-delete-if.  It seems there
> is a long history about not using the functions from cl.el at runtime.

It is fine to use cl-lib at runtime, so no problem.

>  - I removed two strange ^L characters in gdb-mi.el.  Am I right to
> remove them? 

No. (And as a general comment don't mix unrelated formatting changes
with commits.)

> Do they mean anything, or are they garbage ASCII
> character that got there by mistake?

They are page separators. See (emacs)Pages.

>  - When doing my commit, do I need to specify a bug tracker with the
> --fixes option, or is "--fixes 13845" enough?

Add an entry to ~/.bazaar/bazaar.conf:

  [DEFAULT]
  bugtracker_debbugs_url = http://debbugs.gnu.org/{id}

then use --fixes debbugs:13845

bug#13845: 24.3.50; gud commands stop responding when using gdb-mi

[Jean-Philippe Gravel]

[-- Attachment #1: Type: text/plain, Size: 236 bytes --]

Glenn Morris wrote:

> Could you send the change you propose to install to this address as a
> flat diff? I'm assuming it is not enormous.

Certainly.  I divided my branch into three distinct patches that could
be committed separately.

[-- Attachment #2: invalidSignals.patch --]
[-- Type: text/x-patch, Size: 2073 bytes --]

=== modified file 'lisp/ChangeLog'
*** lisp/ChangeLog	2013-04-30 16:27:36 +0000
--- lisp/ChangeLog	2013-05-01 01:17:37 +0000
***************
*** 1,3 ****
--- 1,8 ----
+ 2013-04-30  Jean-Philippe Gravel  <jpgravel@gmail.com>
+ 
+ 	* progmodes/gdb-mi.el (gdb-running, gdb-starting): Removed
+ 	signals for which replies were never received.
+ 
  2013-04-30  Leo Liu  <sdl.web@gmail.com>
  
  	* progmodes/octave.el (inferior-octave-prompt-read-only): Fix last

=== modified file 'lisp/progmodes/gdb-mi.el'
*** lisp/progmodes/gdb-mi.el	2013-04-20 16:24:04 +0000
--- lisp/progmodes/gdb-mi.el	2013-05-01 01:11:33 +0000
***************
*** 2293,2300 ****
     (propertize gdb-inferior-status 'face font-lock-type-face))
    (when (not gdb-non-stop)
      (setq gud-running t))
!   (setq gdb-active-process t)
!   (gdb-emit-signal gdb-buf-publisher 'update-threads))
  
  (defun gdb-starting (_output-field _result)
    ;; CLI commands don't emit ^running at the moment so use gdb-running too.
--- 2293,2299 ----
     (propertize gdb-inferior-status 'face font-lock-type-face))
    (when (not gdb-non-stop)
      (setq gud-running t))
!   (setq gdb-active-process t))
  
  (defun gdb-starting (_output-field _result)
    ;; CLI commands don't emit ^running at the moment so use gdb-running too.
***************
*** 2302,2312 ****
    (gdb-force-mode-line-update
     (propertize gdb-inferior-status 'face font-lock-type-face))
    (setq gdb-active-process t)
!   (setq gud-running t)
!   ;; GDB doesn't seem to respond to -thread-info before first stop or
!   ;; thread exit (even in non-stop mode), so this is useless.
!   ;; Behavior may change in the future.
!   (gdb-emit-signal gdb-buf-publisher 'update-threads))
  
  ;; -break-insert -t didn't give a reason before gdb 6.9
  
--- 2301,2307 ----
    (gdb-force-mode-line-update
     (propertize gdb-inferior-status 'face font-lock-type-face))
    (setq gdb-active-process t)
!   (setq gud-running t))
  
  ;; -break-insert -t didn't give a reason before gdb 6.9
  


[-- Attachment #3: bug13845.patch --]
[-- Type: text/x-patch, Size: 23785 bytes --]

=== modified file 'lisp/ChangeLog'
*** lisp/ChangeLog	2013-04-30 16:27:36 +0000
--- lisp/ChangeLog	2013-05-01 01:43:33 +0000
***************
*** 1,3 ****
--- 1,29 ----
+ 2013-04-28  Jean-Philippe Gravel  <jpgravel@gmail.com>
+ 
+ 	* progmodes/gdb-mi.el: Fix non-responsive gud commands (bug#13845)
+ 	(gdb-handler-alist, gdb-handler-number): Remove variables.
+ 	(gdb-handler-list): New variable.
+ 	(gdb-add-handler, gdb-delete-handler, gdb-get-handler-function)
+ 	(gdb-pending-handler-p, gdb-handle-reply)
+ 	(gdb-remove-all-pending-triggers): New functions.
+ 	(gdb-discard-unordered-replies): New defcustom.
+ 	(gdb-handler): New defstruct.
+ 	(gdb-wait-for-pending): Fix invalid backquote. Use gdb-handler-list.
+ 	instead of gdb-pending-triggers. Update docstring.
+ 	(gdb-init-1): Remove dead variables.  Initialize gdb-handler-list.
+ 	(gdb-speedbar-update, gdb-speedbar-timer-fn, gdb-var-update)
+ 	(gdb-var-update-handler, def-gdb-auto-update-trigger)
+ 	(def-gdb-auto-update-handler, gdb-get-changed-registers)
+ 	(gdb-changed-registers-handler, gdb-get-main-selected-frame)
+ 	(gdb-frame-handler): Pending triggers are now automatically managed.
+ 	(def-gdb-trigger-and-handler, def-gdb-auto-update-handler):
+ 	Remove argument.
+ 	(gdb-input): Automatically handles pending triggers. Update docstring.
+ 	(gdb-resync): Replaced gdb-pending-triggers by gdb-handler-list.
+ 	(gdb-thread-exited, gdb-thread-selected, gdb-register-names-handler):
+ 	Update comments.
+ 	(gdb-done-or-error): Now use gdb-handle-reply.
+ 
  2013-04-30  Leo Liu  <sdl.web@gmail.com>
  
  	* progmodes/octave.el (inferior-octave-prompt-read-only): Fix last

=== modified file 'lisp/progmodes/gdb-mi.el'
*** lisp/progmodes/gdb-mi.el	2013-04-20 16:24:04 +0000
--- lisp/progmodes/gdb-mi.el	2013-05-01 01:51:19 +0000
***************
*** 91,97 ****
  (require 'gud)
  (require 'json)
  (require 'bindat)
! (eval-when-compile (require 'cl-lib))
  
  (declare-function speedbar-change-initial-expansion-list
                    "speedbar" (new-default))
--- 91,97 ----
  (require 'gud)
  (require 'json)
  (require 'bindat)
! (require 'cl-lib)
  
  (declare-function speedbar-change-initial-expansion-list
                    "speedbar" (new-default))
***************
*** 206,213 ****
  (defvar gdb-last-command nil)
  (defvar gdb-prompt-name nil)
  (defvar gdb-token-number 0)
! (defvar gdb-handler-alist '())
! (defvar gdb-handler-number nil)
  (defvar gdb-source-file-list nil
    "List of source files for the current executable.")
  (defvar gdb-first-done-or-error t)
--- 206,213 ----
  (defvar gdb-last-command nil)
  (defvar gdb-prompt-name nil)
  (defvar gdb-token-number 0)
! (defvar gdb-handler-list '()
!   "List of gdb-handler keeping track of all pending GDB commands.")
  (defvar gdb-source-file-list nil
    "List of source files for the current executable.")
  (defvar gdb-first-done-or-error t)
***************
*** 242,274 ****
  	       disposition of output generated by commands that
  	       gdb mode sends to gdb on its own behalf.")
  
! ;; Pending triggers prevent congestion: Emacs won't send two similar
! ;; consecutive requests.
! 
! (defvar gdb-pending-triggers '()
!   "A list of trigger functions which have not yet been handled.
! 
! Elements are either function names or pairs (buffer . function)")
  
! (defmacro gdb-add-pending (item)
!   `(push ,item gdb-pending-triggers))
! (defmacro gdb-pending-p (item)
!   `(member ,item gdb-pending-triggers))
! (defmacro gdb-delete-pending (item)
!   `(setq gdb-pending-triggers
!          (delete ,item gdb-pending-triggers)))
  
  (defmacro gdb-wait-for-pending (&rest body)
!   "Wait until `gdb-pending-triggers' is empty and evaluate FORM.
  
! This function checks `gdb-pending-triggers' value every
! `gdb-wait-for-pending' seconds."
!   (run-with-timer
!    0.5 nil
!    `(lambda ()
!       (if (not gdb-pending-triggers)
!           (progn ,@body)
!         (gdb-wait-for-pending ,@body)))))
  
  ;; Publish-subscribe
  
--- 242,355 ----
  	       disposition of output generated by commands that
  	       gdb mode sends to gdb on its own behalf.")
  
! (defcustom gdb-discard-unordered-replies t
!   "When non-nil, any out-of-order GDB replies are discarded.
! This mechanism is provided as a protection against lost GDB replies,
! assuming that GDB always replies in the same order Emacs is sending
! commands.  When receiving a reply with a given token-number, any
! pending messages with a lower token-number are considered out-of-order."
!   :type 'boolean
!   :group 'gud
!   :version "24.4")
  
! (cl-defstruct gdb-handler
!   "Structure used to keep track of the commands sent to GDB and
! handle the replies received."
!   ;; Prefix of the command sent to GDB.  The GDB reply for this command
!   ;; will be prefixed with this same TOKEN-NUMBER
!   (token-number nil :read-only t)
!   ;; Callback to invoke when the reply is received from GDB
!   (function nil :read-only t)
!   ;; PENDING-TRIGGER is used to prevent congestion: Emacs won't send
!   ;; two requests with the same PENDING-TRIGGER until a reply is received
!   ;; for the first one."
!   (pending-trigger nil))
! 
! (defun gdb-add-handler (token-number handler-function &optional pending-trigger)
!   "Insert a new GDB command handler in `gdb-handler-list'.
! Handlers are used to keep track the commands sent to GDB and to handle the
! replies received.  TOKEN-NUMBER is the number used as prefix of the GDB/mi
! command sent.  Upon reception of a reply prefixed with the same TOKEN-NUMBER,
! the callback HANDLER-FUNCTION is invoked.  If PENDING-TRIGGER is specified,
! no new GDB commands will be sent with this same PENDING-TRIGGER until a reply is
! received for this handler."
! 
!   (push (make-gdb-handler :token-number token-number
!                           :function handler-function
!                           :pending-trigger pending-trigger)
!         gdb-handler-list))
! 
! (defun gdb-delete-handler (token-number)
!   "Remove the handler TOKEN-NUMBER from `gdb-handler-list'.
! Additionally, if `gdb-discard-unordered-replies' is non-nil, all handlers
! having a token number less than TOKEN-NUMBER are discarded."
!   (if gdb-discard-unordered-replies
! 
!       (setq gdb-handler-list
!             (cl-delete-if
!              (lambda (handler)
!                "Discard any HANDLER with a token number `<=' than TOKEN-NUMBER."
!                (when (< (gdb-handler-token-number handler) token-number)
!                  (message (format
!                            "WARNING! Discarding GDB handler with token #%d\n"
!                            (gdb-handler-token-number handler))))
!                (<= (gdb-handler-token-number handler) token-number))
!              gdb-handler-list))
! 
!     (setq gdb-handler-list
!           (cl-delete-if
!            (lambda (handler)
!              "Discard any HANDLER with a token number `eq' to TOKEN-NUMBER."
!              (eq (gdb-handler-token-number handler) token-number))
!            gdb-handler-list))))
! 
! (defun gdb-get-handler-function (token-number)
!   "Return the function callback registered with the handler TOKEN-NUMBER."
!   (gdb-handler-function
!    (cl-find-if (lambda (handler) (eq (gdb-handler-token-number handler)
!                                       token-number))
!                 gdb-handler-list)))
! 
! 
! (defun gdb-pending-handler-p (pending-trigger)
!   "Return non-nil if a command handler is pending with trigger PENDING-TRIGGER."
!   (cl-find-if (lambda (handler) (eq (gdb-handler-pending-trigger handler)
!                                      pending-trigger))
!                gdb-handler-list))
! 
! 
! (defun gdb-handle-reply (token-number)
!   "Handle the GDB reply TOKEN-NUMBER.
! This involves invoking the callback registered with this token number
! in `gdb-handler-list' and clearing all pending handlers invalidated
! by the reception of this reply."
!   (let ((handler-function (gdb-get-handler-function token-number)))
!     (when handler-function
!       (funcall handler-function)
!       (gdb-delete-handler token-number))))
! 
! (defun gdb-remove-all-pending-triggers ()
!   "Remove all pending triggers from gdb-handler-list.
! The handlers are left in gdb-handler-list so that replies received
! from GDB could still be handled.  However, removing the pending triggers
! allows Emacs to send new commands even if replies of previous commands
! were not yet received."
!   (dolist (handler gdb-handler-list)
!     (setf (gdb-handler-pending-trigger handler) nil)))
  
  (defmacro gdb-wait-for-pending (&rest body)
!   "Wait for all pending GDB commands to finish and evaluate BODY.
  
! This function checks every 0.5 seconds if there are any pending
! triggers in `gdb-handler-list'."
!   `(run-with-timer
!     0.5 nil
!     '(lambda ()
!        (if (not (gdb-find-if (lambda (handler)
!                                (gdb-handler-pending-trigger handler))
!                              gdb-handler-list))
! 	   (progn ,@body)
! 	 (gdb-wait-for-pending ,@body)))))
  
  ;; Publish-subscribe
  
***************
*** 820,833 ****
  	gdb-frame-number nil
          gdb-thread-number nil
  	gdb-var-list nil
- 	gdb-pending-triggers nil
  	gdb-output-sink 'user
  	gdb-location-alist nil
  	gdb-source-file-list nil
  	gdb-last-command nil
  	gdb-token-number 0
! 	gdb-handler-alist '()
! 	gdb-handler-number nil
  	gdb-prompt-name nil
  	gdb-first-done-or-error t
  	gdb-buffer-fringe-width (car (window-fringes))
--- 901,912 ----
  	gdb-frame-number nil
          gdb-thread-number nil
  	gdb-var-list nil
  	gdb-output-sink 'user
  	gdb-location-alist nil
  	gdb-source-file-list nil
  	gdb-last-command nil
  	gdb-token-number 0
! 	gdb-handler-list '()
  	gdb-prompt-name nil
  	gdb-first-done-or-error t
  	gdb-buffer-fringe-width (car (window-fringes))
***************
*** 1107,1123 ****
        (message-box "No symbol \"%s\" in current context." expr))))
  
  (defun gdb-speedbar-update ()
!   (when (and (boundp 'speedbar-frame) (frame-live-p speedbar-frame)
! 	     (not (gdb-pending-p 'gdb-speedbar-timer)))
      ;; Dummy command to update speedbar even when idle.
!     (gdb-input "-environment-pwd" 'gdb-speedbar-timer-fn)
!     ;; Keep gdb-pending-triggers non-nil till end.
!     (gdb-add-pending 'gdb-speedbar-timer)))
  
  (defun gdb-speedbar-timer-fn ()
    (if gdb-speedbar-auto-raise
        (raise-frame speedbar-frame))
-   (gdb-delete-pending 'gdb-speedbar-timer)
    (speedbar-timer-fn))
  
  (defun gdb-var-evaluate-expression-handler (varnum changed)
--- 1186,1200 ----
        (message-box "No symbol \"%s\" in current context." expr))))
  
  (defun gdb-speedbar-update ()
!   (when (and (boundp 'speedbar-frame) (frame-live-p speedbar-frame))
      ;; Dummy command to update speedbar even when idle.
!     (gdb-input "-environment-pwd"
!                'gdb-speedbar-timer-fn
!                'gdb-speedbar-update)))
  
  (defun gdb-speedbar-timer-fn ()
    (if gdb-speedbar-auto-raise
        (raise-frame speedbar-frame))
    (speedbar-timer-fn))
  
  (defun gdb-var-evaluate-expression-handler (varnum changed)
***************
*** 1207,1215 ****
  
                                          ; Uses "-var-update --all-values".  Needs GDB 6.4 onwards.
  (defun gdb-var-update ()
!   (if (not (gdb-pending-p 'gdb-var-update))
!       (gdb-input "-var-update --all-values *" 'gdb-var-update-handler))
!   (gdb-add-pending 'gdb-var-update))
  
  (defun gdb-var-update-handler ()
    (let ((changelist (bindat-get-field (gdb-json-partial-output) 'changelist)))
--- 1284,1292 ----
  
                                          ; Uses "-var-update --all-values".  Needs GDB 6.4 onwards.
  (defun gdb-var-update ()
!   (gdb-input "-var-update --all-values *"
!              'gdb-var-update-handler
!              'gdb-var-update))
  
  (defun gdb-var-update-handler ()
    (let ((changelist (bindat-get-field (gdb-json-partial-output) 'changelist)))
***************
*** 1272,1279 ****
                    (push var1 var-list))
                  (setq var1 (pop temp-var-list)))
                (setq gdb-var-list (nreverse var-list))))))))
-   (setq gdb-pending-triggers
- 	(delq 'gdb-var-update gdb-pending-triggers))
    (gdb-speedbar-update))
  
  (defun gdb-speedbar-expand-node (text token indent)
--- 1349,1354 ----
***************
*** 1727,1744 ****
    (setq string (replace-regexp-in-string "\n" "\\n" string t t))
    (concat "\"" string "\""))
  
! (defun gdb-input (command handler-function)
    "Send COMMAND to GDB via the MI interface.
  Run the function HANDLER-FUNCTION, with no arguments, once the command is
! complete."
!   (if gdb-enable-debug (push (list 'send-item command handler-function)
! 			     gdb-debug-log))
!   (setq gdb-token-number (1+ gdb-token-number))
!   (setq command (concat (number-to-string gdb-token-number) command))
!   (push (cons gdb-token-number handler-function) gdb-handler-alist)
!   (if gdbmi-debug-mode (message "gdb-input: %s" command))
!   (process-send-string (get-buffer-process gud-comint-buffer)
! 		       (concat command "\n")))
  
  ;; NOFRAME is used for gud execution control commands
  (defun gdb-current-context-command (command)
--- 1802,1826 ----
    (setq string (replace-regexp-in-string "\n" "\\n" string t t))
    (concat "\"" string "\""))
  
! (defun gdb-input (command handler-function &optional trigger-name)
    "Send COMMAND to GDB via the MI interface.
  Run the function HANDLER-FUNCTION, with no arguments, once the command is
! complete.  If TRIGGER-NAME is non-nil, the COMMAND will not be sent to GDB
! if Emacs is still waiting for a reply from another command was previously
! sent with the same TRIGGER-NAME."
!   (when (or (not trigger-name)
!             (not (gdb-pending-handler-p trigger-name)))
! 
!     (if gdb-enable-debug (push (list 'send-item command handler-function)
!                                gdb-debug-log))
!     (setq gdb-token-number (1+ gdb-token-number))
!     (setq command (concat (number-to-string gdb-token-number) command))
! 
!     (gdb-add-handler gdb-token-number handler-function trigger-name)
! 
!     (if gdbmi-debug-mode (message "gdb-input: %s" command))
!     (process-send-string (get-buffer-process gud-comint-buffer)
!                          (concat command "\n"))))
  
  ;; NOFRAME is used for gud execution control commands
  (defun gdb-current-context-command (command)
***************
*** 1774,1780 ****
  (defun gdb-resync()
    (setq gud-running nil)
    (setq gdb-output-sink 'user)
!   (setq gdb-pending-triggers nil))
  
  (defun gdb-update (&optional no-proc)
    "Update buffers showing status of debug session.
--- 1856,1862 ----
  (defun gdb-resync()
    (setq gud-running nil)
    (setq gdb-output-sink 'user)
!   (gdb-remove-all-pending-triggers))
  
  (defun gdb-update (&optional no-proc)
    "Update buffers showing status of debug session.
***************
*** 2255,2263 ****
      (if (string= gdb-thread-number thread-id)
          (gdb-setq-thread-number nil))
      ;; When we continue current thread and it quickly exits,
!     ;; gdb-pending-triggers left after gdb-running disallow us to
!     ;; properly call -thread-info without --thread option. Thus we
!     ;; need to use gdb-wait-for-pending.
      (gdb-wait-for-pending
       (gdb-emit-signal gdb-buf-publisher 'update-threads))))
  
--- 2337,2345 ----
      (if (string= gdb-thread-number thread-id)
          (gdb-setq-thread-number nil))
      ;; When we continue current thread and it quickly exits,
!     ;; the pending triggers in gdb-handler-list left after gdb-running
!     ;; disallow us to properly call -thread-info without --thread option.
!     ;; Thus we need to use gdb-wait-for-pending.
      (gdb-wait-for-pending
       (gdb-emit-signal gdb-buf-publisher 'update-threads))))
  
***************
*** 2272,2280 ****
      ;; by `=thread-selected` notification. `^done` causes `gdb-update`
      ;; as usually. Things happen to fast and second call (from
      ;; gdb-thread-selected handler) gets cut off by our beloved
!     ;; gdb-pending-triggers.
!     ;; Solution is `gdb-wait-for-pending` macro: it guarantees that its
!     ;; body will get executed when `gdb-pending-triggers` is empty.
      (gdb-wait-for-pending
       (gdb-update))))
  
--- 2354,2363 ----
      ;; by `=thread-selected` notification. `^done` causes `gdb-update`
      ;; as usually. Things happen to fast and second call (from
      ;; gdb-thread-selected handler) gets cut off by our beloved
!     ;; pending triggers.
!     ;; Solution is `gdb-wait-for-pending' macro: it guarantees that its
!     ;; body will get executed when `gdb-handler-list' if free of
!     ;; pending triggers.
      (gdb-wait-for-pending
       (gdb-update))))
  
***************
*** 2438,2447 ****
      (when (and token-number is-complete)
        (with-current-buffer
  	  (gdb-get-buffer-create 'gdb-partial-output-buffer)
! 	(funcall
! 	 (cdr (assoc (string-to-number token-number) gdb-handler-alist))))
!       (setq gdb-handler-alist
!             (assq-delete-all token-number gdb-handler-alist)))
  
    (when is-complete
      (gdb-clear-partial-output))))
--- 2521,2527 ----
      (when (and token-number is-complete)
        (with-current-buffer
  	  (gdb-get-buffer-create 'gdb-partial-output-buffer)
! 	(gdb-handle-reply (string-to-number token-number))))
  
    (when is-complete
      (gdb-clear-partial-output))))
***************
*** 2659,2685 ****
       (when
           (or (not ,signal-list)
               (memq signal ,signal-list))
!        (when (not (gdb-pending-p
!                    (cons (current-buffer) ',trigger-name)))
!          (gdb-input ,gdb-command
! 		    (gdb-bind-function-to-buffer ',handler-name (current-buffer)))
!          (gdb-add-pending (cons (current-buffer) ',trigger-name))))))
  
  ;; Used by disassembly buffer only, the rest use
  ;; def-gdb-trigger-and-handler
! (defmacro def-gdb-auto-update-handler (handler-name trigger-name custom-defun
                                                      &optional nopreserve)
!   "Define a handler HANDLER-NAME for TRIGGER-NAME with CUSTOM-DEFUN.
  
  Handlers are normally called from the buffers they put output in.
  
! Delete ((current-buffer) . TRIGGER-NAME) from
! `gdb-pending-triggers', erase current buffer and evaluate
! CUSTOM-DEFUN.  Then `gdb-update-buffer-name' is called.
  
  If NOPRESERVE is non-nil, window point is not restored after CUSTOM-DEFUN."
    `(defun ,handler-name ()
-      (gdb-delete-pending (cons (current-buffer) ',trigger-name))
       (let* ((inhibit-read-only t)
              ,@(unless nopreserve
                  '((window (get-buffer-window (current-buffer) 0))
--- 2739,2761 ----
       (when
           (or (not ,signal-list)
               (memq signal ,signal-list))
!        (gdb-input ,gdb-command
!                   (gdb-bind-function-to-buffer ',handler-name (current-buffer))
!                   (cons (current-buffer) ',trigger-name)))))
  
  ;; Used by disassembly buffer only, the rest use
  ;; def-gdb-trigger-and-handler
! (defmacro def-gdb-auto-update-handler (handler-name custom-defun
                                                      &optional nopreserve)
!   "Define a handler HANDLER-NAME calling CUSTOM-DEFUN.
  
  Handlers are normally called from the buffers they put output in.
  
! Erase current buffer and evaluate CUSTOM-DEFUN.
! Then `gdb-update-buffer-name' is called.
  
  If NOPRESERVE is non-nil, window point is not restored after CUSTOM-DEFUN."
    `(defun ,handler-name ()
       (let* ((inhibit-read-only t)
              ,@(unless nopreserve
                  '((window (get-buffer-window (current-buffer) 0))
***************
*** 2707,2713 ****
         ,gdb-command
         ,handler-name ,signal-list)
       (def-gdb-auto-update-handler ,handler-name
!        ,trigger-name ,custom-defun)))
  
  \f
  
--- 2783,2789 ----
         ,gdb-command
         ,handler-name ,signal-list)
       (def-gdb-auto-update-handler ,handler-name
!        ,custom-defun)))
  
  \f
  
***************
*** 3624,3630 ****
  
  (def-gdb-auto-update-handler
    gdb-disassembly-handler
-   gdb-invalidate-disassembly
    gdb-disassembly-handler-custom
    t)
  
--- 3700,3705 ----
***************
*** 4116,4136 ****
  
  ;; Needs GDB 6.4 onwards (used to fail with no stack).
  (defun gdb-get-changed-registers ()
!   (when (and (gdb-get-buffer 'gdb-registers-buffer)
! 	     (not (gdb-pending-p 'gdb-get-changed-registers)))
      (gdb-input "-data-list-changed-registers"
! 	       'gdb-changed-registers-handler)
!     (gdb-add-pending 'gdb-get-changed-registers)))
  
  (defun gdb-changed-registers-handler ()
-   (gdb-delete-pending 'gdb-get-changed-registers)
    (setq gdb-changed-registers nil)
    (dolist (register-number
             (bindat-get-field (gdb-json-partial-output) 'changed-registers))
      (push register-number gdb-changed-registers)))
  
  (defun gdb-register-names-handler ()
!   ;; Don't use gdb-pending-triggers because this handler is called
    ;; only once (in gdb-init-1)
    (setq gdb-register-names nil)
    (dolist (register-name
--- 4191,4209 ----
  
  ;; Needs GDB 6.4 onwards (used to fail with no stack).
  (defun gdb-get-changed-registers ()
!   (when (gdb-get-buffer 'gdb-registers-buffer)
      (gdb-input "-data-list-changed-registers"
!                'gdb-changed-registers-handler
!                'gdb-get-changed-registers)))
  
  (defun gdb-changed-registers-handler ()
    (setq gdb-changed-registers nil)
    (dolist (register-number
             (bindat-get-field (gdb-json-partial-output) 'changed-registers))
      (push register-number gdb-changed-registers)))
  
  (defun gdb-register-names-handler ()
!   ;; Don't use pending triggers because this handler is called
    ;; only once (in gdb-init-1)
    (setq gdb-register-names nil)
    (dolist (register-name
***************
*** 4154,4169 ****
  (defun gdb-get-main-selected-frame ()
    "Trigger for `gdb-frame-handler' which uses main current thread.
  Called from `gdb-update'."
!   (if (not (gdb-pending-p 'gdb-get-main-selected-frame))
!       (progn
! 	(gdb-input (gdb-current-context-command "-stack-info-frame")
! 		   'gdb-frame-handler)
! 	(gdb-add-pending 'gdb-get-main-selected-frame))))
  
  (defun gdb-frame-handler ()
    "Set `gdb-selected-frame' and `gdb-selected-file' to show
  overlay arrow in source buffer."
-   (gdb-delete-pending 'gdb-get-main-selected-frame)
    (let ((frame (bindat-get-field (gdb-json-partial-output) 'frame)))
      (when frame
        (setq gdb-selected-frame (bindat-get-field frame 'func))
--- 4227,4239 ----
  (defun gdb-get-main-selected-frame ()
    "Trigger for `gdb-frame-handler' which uses main current thread.
  Called from `gdb-update'."
!   (gdb-input (gdb-current-context-command "-stack-info-frame")
!              'gdb-frame-handler
!              'gdb-get-main-selected-frame))
  
  (defun gdb-frame-handler ()
    "Set `gdb-selected-frame' and `gdb-selected-file' to show
  overlay arrow in source buffer."
    (let ((frame (bindat-get-field (gdb-json-partial-output) 'frame)))
      (when frame
        (setq gdb-selected-frame (bindat-get-field frame 'func))


[-- Attachment #4: tokenNumber.patch --]
[-- Type: text/x-patch, Size: 1788 bytes --]

=== modified file 'lisp/ChangeLog'
*** lisp/ChangeLog	2013-04-30 16:27:36 +0000
--- lisp/ChangeLog	2013-05-01 01:29:01 +0000
***************
*** 1,3 ****
--- 1,8 ----
+ 2013-04-30  Jean-Philippe Gravel  <jpgravel@gmail.com>
+ 
+ 	* progmodes/gdb-mi.el (gdb-input): Include token numbers in
+ 	gdb-debug-log.
+ 
  2013-04-30  Leo Liu  <sdl.web@gmail.com>
  
  	* progmodes/octave.el (inferior-octave-prompt-read-only): Fix last

=== modified file 'lisp/progmodes/gdb-mi.el'
*** lisp/progmodes/gdb-mi.el	2013-04-20 16:24:04 +0000
--- lisp/progmodes/gdb-mi.el	2013-05-01 01:48:31 +0000
***************
*** 1731,1740 ****
    "Send COMMAND to GDB via the MI interface.
  Run the function HANDLER-FUNCTION, with no arguments, once the command is
  complete."
-   (if gdb-enable-debug (push (list 'send-item command handler-function)
- 			     gdb-debug-log))
    (setq gdb-token-number (1+ gdb-token-number))
    (setq command (concat (number-to-string gdb-token-number) command))
    (push (cons gdb-token-number handler-function) gdb-handler-alist)
    (if gdbmi-debug-mode (message "gdb-input: %s" command))
    (process-send-string (get-buffer-process gud-comint-buffer)
--- 1731,1741 ----
    "Send COMMAND to GDB via the MI interface.
  Run the function HANDLER-FUNCTION, with no arguments, once the command is
  complete."
    (setq gdb-token-number (1+ gdb-token-number))
    (setq command (concat (number-to-string gdb-token-number) command))
+ 
+   (if gdb-enable-debug (push (list 'send-item command handler-function)
+ 			     gdb-debug-log))
    (push (cons gdb-token-number handler-function) gdb-handler-alist)
    (if gdbmi-debug-mode (message "gdb-input: %s" command))
    (process-send-string (get-buffer-process gud-comint-buffer)

bug#13845: 24.3.50; gud commands stop responding when using gdb-mi

[Glenn Morris]

Thanks. I'm not familiar with gdb, so these are style comments only.

Jean-Philippe Gravel wrote:

> + 	* progmodes/gdb-mi.el (gdb-running, gdb-starting): Removed
> + 	signals for which replies were never received.

Use the present tense for ChangeLog entries.

> + 	(gdb-wait-for-pending): Fix invalid backquote. Use gdb-handler-list.
> + 	instead of gdb-pending-triggers. Update docstring.

Two spaces between sentences.

> ! (defcustom gdb-discard-unordered-replies t
> !   "When non-nil, any out-of-order GDB replies are discarded.

Use the active voice; eg

  Non-nil means discard any out-of-order GDB replies.

> ! This mechanism is provided as a protection against lost GDB replies,
> ! assuming that GDB always replies in the same order Emacs is sending
> ! commands.  When receiving a reply with a given token-number, any
> ! pending messages with a lower token-number are considered out-of-order.

  This protects against lost GDB replies, assuming that GDB always
  replies in the same order as Emacs sends commands.  When receiving a
  reply with a given token-number, assume any pending messages with a
  lower token-number are out-of-order.

> ! (cl-defstruct gdb-handler
> !   "Structure used to keep track of the commands sent to GDB and
> ! handle the replies received."

First line of doc should be a complete sentence.

> ! Handlers are used to keep track the commands sent to GDB and to handle the

Remove "keep", or add "of" (keep track of).

> ! replies received.  TOKEN-NUMBER is the number used as prefix of the GDB/mi
> ! command sent. 

Active voice: Use TOKEN-NUMBER as the prefix...

> ! Upon reception of a reply prefixed with the same TOKEN-NUMBER,
> ! the callback HANDLER-FUNCTION is invoked.

active: invoke the callback...

> ! Additionally, if `gdb-discard-unordered-replies' is non-nil, all handlers
> ! having a token number less than TOKEN-NUMBER are discarded."

active: discard all handlers...

> ! This involves invoking the callback registered with this token number
> ! in `gdb-handler-list' and clearing all pending handlers invalidated
> ! by the reception of this reply."

This invokes the handler ... and clears all pending...

bug#13845: 24.3.50; gud commands stop responding when using gdb-mi

[Stefan Monnier]

> Thanks. I'm not familiar with gdb, so these are style comments only.

Other than Glenn's comments, it looks fine, thank you.
Just one more detail:

The text in the ChangeLog files should ideally be redundant (i.e. we
should be able to regenerate it from the commit log), so please make
sure you use the same text for the commit messages (tho with the leading
TABs removed) as the text you wrote in the ChangeLog.

If you commit from vc-dir you should be able to use C-c C-a from the
*VC-Log* buffer to grab the commit message from the ChangeLog files
(tho it seems it currently only works if you tell VC exactly which files
you want to commit, rather than just telling it to commit the top
directory).


        Stefan

bug#13845: 24.3.50; gud commands stop responding when using gdb-mi

[Jean-Philippe Gravel]

Hi Glenn and Stefan,

Thank's for the review.  I will make the correction you propose and go
ahead with the commit.

Jean-Philippe

On Sun, May 5, 2013 at 1:28 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> Thanks. I'm not familiar with gdb, so these are style comments only.
>
> Other than Glenn's comments, it looks fine, thank you.
> Just one more detail:
>
> The text in the ChangeLog files should ideally be redundant (i.e. we
> should be able to regenerate it from the commit log), so please make
> sure you use the same text for the commit messages (tho with the leading
> TABs removed) as the text you wrote in the ChangeLog.
>
> If you commit from vc-dir you should be able to use C-c C-a from the
> *VC-Log* buffer to grab the commit message from the ChangeLog files
> (tho it seems it currently only works if you tell VC exactly which files
> you want to commit, rather than just telling it to commit the top
> directory).
>
>
>         Stefan

bug#13845: 24.3.50; gud commands stop responding when using gdb-mi

[Jean-Philippe Gravel]

> Thank's for the review.  I will make the correction you propose and go
> ahead with the commit.

Done.

bug#13845: 24.3.50; gud commands stop responding when using gdb-mi

[Glenn Morris]

Jean-Philippe Gravel wrote:

>> Thank's for the review.  I will make the correction you propose and go
>> ahead with the commit.
>
> Done.

Thanks. As final step, please close this report, by sending a mail to
13845-done@debbugs.

bug#13845: 24.3.50; gud commands stop responding when using gdb-mi

[Jean-Philippe Gravel]

Closing report.