emacsclient wrapper: introduce more uses of timeouts

Also improve handling of exit codes of intermediate gdbmacs requests.
Where these use gdb-wait-for-pending, if they don't exit zero, it means we
couldn't even submit the request, not just that it couldn't be fulfilled.

Also fix inotifywait(1) event type delete => delete_self.

1 files changed, 75 insertions, 35 deletions

diff --git a/bin/emacsclient b/bin/emacsclient
index 16aca4ae..8cb4b75b 100755
--- a/bin/emacsclient
+++ b/bin/emacsclient
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-# Copyright (C) 2022  Sean Whitton
+# Copyright (C) 2022-2023  Sean Whitton
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -26,6 +26,7 @@ want_update=false
 want_installed=false
 want_eval=false
 want_version=false
+timeout=
 devel_running=false
 installed_running=false
 make=
@@ -71,16 +72,23 @@ fail () {
     maybe_notify "$1"; exit 1
 }
 
+wait_inotifywait () {
+    wait $inotifywait
+    case $? in
+	0|2) return $? ;;
+	*) fail "inotifywait(1) exited with code $?" ;;
+    esac
+}
+
 spw_flock () {
     ( umask 077; mkdir -p "$locks_dir" )
     eval "exec $1<>${locks_dir}$2"
-    flock --wait 15 "$1" \
+    flock $([ -n "$timeout" ] && printf -- "--wait %s" "$timeout") "$1" \
 	|| fail "couldn't lock starting Emacs daemon named $2"
 }
 
 pass_to_gdbmacs () {
     if $want_eval || $want_update; then
-	maybe_notify "in-tree Emacs appears to be wedged"
 	# Only exit non-zero if we were expected to start the daemon.
 	! $may_start; exit
     else
@@ -104,9 +112,24 @@ for arg do
 	'--spw/update-environment')      want_update=true	 ;;
 	'-V'|'--version')                want_version=true	 ;;&
 	'-e'|'--eval')                   want_eval=true          ;;&
+
+	# We'll use the same timeout for some things we do in this script,
+	# if the user has supplied a value.
+	# Otherwise, we wait forever, just like emacsclient(1), for flock(1).
+	#
+	# For inotifywait(1) and our own --eval requests, we always supply
+	# timeouts.  This is because these requests can fail for reasons this
+	# script can't detect, and we don't want to wait forever while holding
+	# locks on starting up daemons, else no other invocation of this
+	# script will be able to do anything without manual intervention.
+	'-w'|'--timeout')                timeout="$1"            ;;&
+	'-w'?*)                          timeout="${arg:2}"      ;;&
+	'--timeout='?*)                  timeout="${arg:10}"     ;;&
+
 	'-s'|'--socket-name') min_arg=2; daemon_name="$1"        ;;&
 	'-s'?*)               min_arg=1; daemon_name="${arg:2}"  ;;&
 	'--socket-name='?*)   min_arg=1; daemon_name="${arg:14}" ;;&
+
 	*) set -- "$@" "$arg" ;;
     esac
 done
@@ -140,22 +163,25 @@ fi
 # Make it possible, with primary session, to quickly replace in-tree Emacs
 # with installed Emacs.  See 'C-i E' Sway/i3 binding.
 if [ -z "$daemon_name" ] && $devel_running && $want_installed; then
-    inotifywait -qq -e delete "$socket" &
+    inotifywait -qq --timeout "${timeout:-15}" -e delete_self "$socket" &
     inotifywait=$!
     kill "$listener"
 
     # Detach gdb so that Emacs can handle the SIGTERM.  We also have to quit
     # gdb so that `spw/gdbmacs-attach' doesn't look at `gdb-inferior-status'.
-    [ -n "${gdbmacs:=$(get_listener ${socket_dir}gdbmacs)}" ] \
-	&& ( exec -a emacsclient "$installed_emacsclient" -sgdbmacs \
-		  --eval '(gdb-wait-for-pending
-		           (lambda ()
-		             (gud-basic-call "detach")
-		             (gdb-wait-for-pending
-		              (lambda ()
-		                (gud-basic-call "quit")))))' )
+    if [ -n "${gdbmacs:=$(get_listener ${socket_dir}gdbmacs)}" ]; then \
+	( exec -a emacsclient "$installed_emacsclient" -sgdbmacs -w2 \
+		   --eval '(gdb-wait-for-pending
+			    (lambda ()
+			      (gud-basic-call "detach")
+			      (gdb-wait-for-pending
+			       (lambda ()
+				 (gud-basic-call "quit")))))' ) \
+				     || fail "gdbmacs detach request failed"
+    fi
 
-    wait $inotifywait
+    wait_inotifywait \
+	|| fail "inotifywait(1) timed out waiting for socket deletion"
     listener=
     devel_running=false
 fi
@@ -178,14 +204,29 @@ if ! $want_installed && ! $installed_running && [ -x "$devel_emacsclient" ] \
     emacsclient=("$devel_emacsclient")
     if [ -z "$daemon_name" ] && ! $want_version; then
 	spw_flock 4 gdbmacs
-	[ -n "${gdbmacs:=$(get_listener ${socket_dir}gdbmacs)}" ] \
-	    && gud_status=$(exec -a emacsclient "$installed_emacsclient" \
-				 -sgdbmacs \
-				 --eval \
-				 '(and (boundp '"'"'gud-comint-buffer)
-				       (get-buffer-process gud-comint-buffer)
-				       (string= "signal-received"
-				                gdb-inferior-status))')
+	if [ -n "${gdbmacs:=$(get_listener ${socket_dir}gdbmacs)}" ]; then
+	    # If the primary session is known to be stopped, ask gdbmacs to
+	    # handle the request: it's probably C-i e or editing via EDITOR.
+	    #
+	    # Ignore a failure to obtain any information here, because one
+	    # likely cause is that the request times out because gdbmacs is
+	    # busy starting up Gnus.  In that case we'd rather just try
+	    # sending the request to the primary session anyway.
+	    #
+	    # Ideally we could know that emacsclient exited non-zero only
+	    # because of a timeout, but currently that can be done only by
+	    # parsing emacsclient's output: see Emacs bug#60592.
+	    gud_status="$(exec -a emacsclient "$installed_emacsclient" \
+			       	-sgdbmacs -w2 --eval \
+				'(and (boundp '"'"'gud-comint-buffer)
+				      (get-buffer-process gud-comint-buffer)
+				      (string= "signal-received"
+					       gdb-inferior-status))')"
+	    if [ $? -eq 0 -a "$gud_status" = t ]; then
+		maybe_notify "in-tree Emacs appears to be wedged"
+		pass_to_gdbmacs "$@"
+	    fi
+	fi
     fi
 else
     emacs=(-a emacs "$installed_emacs")
@@ -199,23 +240,22 @@ fi
 # patch the in-tree emacsclient.c to only execute the in-tree Emacs, instead
 # of handling it here.)
 
-if [ "$gud_status" = t ]; then
-    # Primary session is stopped.  Ask gdbmacs to handle the request: it's
-    # probably C-i e or editing a file via EDITOR.
-    pass_to_gdbmacs "$@"
-elif [ -z "$listener" ] && ! $want_version; then
+if [ -z "$listener" ] && ! $want_version; then
     if [ -z "$daemon_name" -a "${emacs[0]}" = "$devel_emacs" ]; then
 	# inotifywait(1) in Debian "bullseye" doesn't have --include.
-	inotifywait -qq -e create "${socket_dir}" \
+	inotifywait -qq --timeout "${timeout:-15}" \
+		    -e create "${socket_dir}" \
 		    --exclude "$(egrep_negate ${socket_dir}server)" &
 	inotifywait=$!
-	# If gdbmacs fails to start main session, have gdbmacs handle the
-	# request; again, probably C-i e or editing via EDITOR.
-	if ( exec -a emacsclient "$installed_emacsclient" -a '' -sgdbmacs \
-		  --eval '(spw/gdbmacs-attach)' 3>&- 4>&- ); then
-	    wait $inotifywait
-	else
-	    kill $inotifywait
+	# If gdbmacs fails to start main session, but we were able to
+	# successfully submit the start request, have gdbmacs handle the
+	# actual request too; again, probably C-i e or editing via EDITOR.
+	( exec -a emacsclient "$installed_emacsclient" -a '' \
+	       -sgdbmacs -w2 --eval '(spw/gdbmacs-attach)' 3>&- 4>&- ) \
+	    || fail "gdbmacs start request failed"
+	if ! wait_inotifywait; then
+	    maybe_notify \
+		"timed out waiting for gdbmacs to start in-tree Emacs"
 	    pass_to_gdbmacs "$@"
 	fi
     else
@@ -242,7 +282,7 @@ elif $want_update && ! $want_version; then
 	fi
     done
     [ -n "$args" ] \
-	&& ( exec "${emacsclient[@]}" -s"${daemon_name:-server}" \
+	&& ( exec "${emacsclient[@]}" -s"${daemon_name:-server}" -w2 \
 		  --eval "(spw/update-environment$args))" )
 fi