bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Jean Louis]

This is wish for the feature or proposal for Emacs daemon to somehow
signal to `emacsclient' to be busy.

We know that if there is no daemon there is environment variable to
spawn some editor instead, which could be like `emacs -Q' or even `vim'

If however, the Emacs daemon spawned by `emacs --bg-daemon' is busy for
example playing video or doing some other process, then `emacsclient'
cannot know about it and is waiting in background.

It would be good to provide some kind of a signal that main Emacs
process is busy so that `emacsclient' can know about it and for example
spawn the alternative editor in that situation. This feature could be
then enabled through customization or variable.




In GNU Emacs 28.0.50 (build 7, x86_64-pc-linux-gnu, X toolkit, cairo version 1.17.4, Xaw3d scroll bars)
 of 2021-09-21 built on protected.rcdrun.com
Repository revision: f23c5c3e445385031d46ee56b89fcda1774a2108
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Parabola GNU/Linux-libre

Configured using:
 'configure --with-x-toolkit=lucid'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY
PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11
XAW3D XDBE XIM XPM LUCID ZLIB

Important settings:
  value of $LC_ALL: en_US.UTF-8
  value of $LANG: de_DE.UTF-8
  value of $XMODIFIERS: @im=exwm-xim
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-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
  indent-tabs-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg rfc6068 epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 54696 7081)
 (symbols 48 7163 3)
 (strings 32 20184 2198)
 (string-bytes 1 644302)
 (vectors 16 14728)
 (vector-slots 8 192430 11692)
 (floats 8 23 45)
 (intervals 56 267 0)
 (buffers 992 12))

-- 
Thanks,
Jean Louis

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Jean Louis <bugs@gnu.support>
> Date: Mon, 27 Sep 2021 17:23:21 +0300
> 
> This is wish for the feature or proposal for Emacs daemon to somehow
> signal to `emacsclient' to be busy.
> 
> We know that if there is no daemon there is environment variable to
> spawn some editor instead, which could be like `emacs -Q' or even `vim'
> 
> If however, the Emacs daemon spawned by `emacs --bg-daemon' is busy for
> example playing video or doing some other process, then `emacsclient'
> cannot know about it and is waiting in background.
> 
> It would be good to provide some kind of a signal that main Emacs
> process is busy so that `emacsclient' can know about it and for example
> spawn the alternative editor in that situation. This feature could be
> then enabled through customization or variable.

The problem in these situations is that AFAIU the daemon doesn't even
know that emacsclient is trying to connect, because it (the daemon)
doesn't process connection requests when it's busy.

You could perhaps do it yourself by deleting the daemon socket when
you play the music.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> The problem in these situations is that AFAIU the daemon doesn't even
> know that emacsclient is trying to connect, because it (the daemon)
> doesn't process connection requests when it's busy.

Perhaps emacsclient could have a timeout here?  I'm not sure what a
reasonable timeout would be, though, but perhaps that can be a command
line switch so that the user can decide.

(I haven't looked at the code; perhaps it has a timeout already.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Jean Louis <bugs@gnu.support>,  50849@debbugs.gnu.org
> Date: Tue, 28 Sep 2021 07:56:17 +0200
> 
> Perhaps emacsclient could have a timeout here?  I'm not sure what a
> reasonable timeout would be, though, but perhaps that can be a command
> line switch so that the user can decide.

There is a (built-in) timeout, but it's way too long AFAIK.  And we
don't handle it any different from any other error: we exit.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> There is a (built-in) timeout, but it's way too long AFAIK.  And we
> don't handle it any different from any other error: we exit.

Doing some testing, it seems to hang here in that recv:

  /* Now, wait for an answer and print any messages.  */
  while (exit_status == EXIT_SUCCESS)
    {
      do
	{
	  act_on_signals (emacs_socket);
	  rl = recv (emacs_socket, string, BUFSIZ, 0);
	}
      while (rl < 0 && errno == EINTR);

Does the recv time out?

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: bugs@gnu.support,  50849@debbugs.gnu.org
> Date: Fri, 02 Sep 2022 13:09:14 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > There is a (built-in) timeout, but it's way too long AFAIK.  And we
> > don't handle it any different from any other error: we exit.
> 
> Doing some testing, it seems to hang here in that recv:
> 
>   /* Now, wait for an answer and print any messages.  */
>   while (exit_status == EXIT_SUCCESS)
>     {
>       do
> 	{
> 	  act_on_signals (emacs_socket);
> 	  rl = recv (emacs_socket, string, BUFSIZ, 0);
> 	}
>       while (rl < 0 && errno == EINTR);
> 
> Does the recv time out?

Looks like it doesn't by default, but we can ask it to do so.  See,
for example,
https://stackoverflow.com/questions/30395258/setting-timeout-to-recv-function.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Looks like it doesn't by default, but we can ask it to do so.  See,
> for example,
> https://stackoverflow.com/questions/30395258/setting-timeout-to-recv-function.

For posterity (in case Stackoverflow goes away before somebody gets
around to fixing this 🫠), the suggested solution is to do a setsockopt
with a RCVTIMEO.  Which seems reasonable.

What would be a reasonable timeout here?  Say...  five seconds?

---
On Windows, the code would look like this:

DWORD timeout = SOCKET_READ_TIMEOUT_SEC * 1000;
setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, (char*)&timeout, sizeof(timeout));

//...

recv_size = recv(s, rx_tmp, bufSize, 0);
if (recv_size == SOCKET_ERROR)
{
    if (WSAGetLastError() != WSAETIMEDOUT)
        //...
}

On other platforms, the code would look like this instead:

struct timeval timeout;
timeout.tv_sec = SOCKET_READ_TIMEOUT_SEC;
timeout.tv_usec = 0;
setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeout));

//...

recv_size = recv(s, rx_tmp, bufSize, 0);
if (recv_size == -1)
{
    if ((errno != EAGAIN) && (errno != EWOULDBLOCK))
        //...
}

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: bugs@gnu.support,  50849@debbugs.gnu.org
> Date: Fri, 02 Sep 2022 14:28:18 +0200
> 
> What would be a reasonable timeout here?  Say...  five seconds?

Too short, IMO.  I'd say 10 (and expose this as a Lisp variable, so
whoever is unhappy about the default can do what they want).

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Robert Pluim]

>>>>> On Fri, 02 Sep 2022 15:39:16 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Lars Ingebrigtsen <larsi@gnus.org>
    >> Cc: bugs@gnu.support,  50849@debbugs.gnu.org
    >> Date: Fri, 02 Sep 2022 14:28:18 +0200
    >> 
    >> What would be a reasonable timeout here?  Say...  five seconds?

    Eli> Too short, IMO.  I'd say 10 (and expose this as a Lisp variable, so
    Eli> whoever is unhappy about the default can do what they want).

10 seconds by default, but with a command line arg, for those times
when youʼve spiked the load average to 1000 😀. And I donʼt see how
exposing it as a Lisp variable helps: the timeout would be on the
emacsclient side, not the Emacs side.

Robert
-- 

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> What would be a reasonable timeout here?  Say...  five seconds?
>
> Too short, IMO.  I'd say 10 (and expose this as a Lisp variable, so
> whoever is unhappy about the default can do what they want).

Makes sense.

But I'm slightly leery about having a default timeout at all -- I can
imagine that there are people using emacsclient as an RPC thing to
control an Emacs automatically, and in that case, any timeout would be
wrong: It's more important to have a reliable mechanism than anything
else.  (And in that scenario, the Emacs daemon might well be busy for a
long time, if you have a script that just feeds commands to the
daemon...)

But on the other hand, it's nice to give the user some more feedback
there than just hanging.

Hm...

Might it make sense to use the timeout to just output a message on
stdout saying "Unable to contact the server; use `C-c' to break" or
something?  Instead of just exiting/erroring out?

Anybody have opinions here?

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Visuwesh]

[வெள்ளி செப்டம்பர் 02, 2022] Lars Ingebrigtsen wrote:

> But I'm slightly leery about having a default timeout at all -- I can
> imagine that there are people using emacsclient as an RPC thing to
> control an Emacs automatically, and in that case, any timeout would be
> wrong: It's more important to have a reliable mechanism than anything
> else.  (And in that scenario, the Emacs daemon might well be busy for a
> long time, if you have a script that just feeds commands to the
> daemon...)
>
> But on the other hand, it's nice to give the user some more feedback
> there than just hanging.
>
> Hm...
>
> Might it make sense to use the timeout to just output a message on
> stdout saying "Unable to contact the server; use `C-c' to break" or
> something?  Instead of just exiting/erroring out?
>
> Anybody have opinions here?

Can we not add a new flag like "--no-timeout" for the people who expect
emacsclient to never time out?  I have a shell script that spawns an
Emacs frame with a new M-x shell buffer every time I call it and it
serves me really well but I have one minor annoyance: there is no way to
fallback to a (boring) terminal emulator if Emacs is busy.
When Emacs is busy and nothing shows up on my screen, I am always
surprised then remember that I let Emacs do something CPU hungry and
pull up dmenu to spawn a st window.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Visuwesh <visuweshm@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  bugs@gnu.support,  50849@debbugs.gnu.org
> Date: Fri, 02 Sep 2022 19:24:05 +0530
> 
> Can we not add a new flag like "--no-timeout" for the people who expect
> emacsclient to never time out?

No, because this will change a long-standing behavior (of waiting
indefinitely).  But we can add a new flag --timeout=NNN to let people
specify a timeout if they want.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Might it make sense to use the timeout to just output a message on
> stdout saying "Unable to contact the server; use `C-c' to break" or
> something?  Instead of just exiting/erroring out?

Yeah, that's a good idea.  Though "Server not responding" sounds like a
better message.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> No, because this will change a long-standing behavior (of waiting
> indefinitely).  But we can add a new flag --timeout=NNN to let people
> specify a timeout if they want.

I don't think a --timeout parameter will be useful here, though --
nobody is going to type "emacsclient --timeout 10 -nw".

So I think an informational message (like previously mentioned) is the
best we can do here.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Lars Ingebrigtsen]

Po Lu <luangruo@yahoo.com> writes:

> Yeah, that's a good idea.  Though "Server not responding" sounds like a
> better message.

Yup.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> No, because this will change a long-standing behavior (of waiting
>> indefinitely).  But we can add a new flag --timeout=NNN to let people
>> specify a timeout if they want.

Hmm, is that so bad though?  I'm not sure I see what problem it could
cause.  Do people actually want and rely on the indefinite waiting?

> I don't think a --timeout parameter will be useful here, though --
> nobody is going to type "emacsclient --timeout 10 -nw".

I often do things like

   alias emacsclient="emacsclient --timeout 10"

so I think such a flag would still be useful.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sat, 3 Sep 2022 08:40:16 -0700
> Cc: 50849@debbugs.gnu.org, bugs@gnu.support, Visuwesh <visuweshm@gmail.com>
> 
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
> >> No, because this will change a long-standing behavior (of waiting
> >> indefinitely).  But we can add a new flag --timeout=NNN to let people
> >> specify a timeout if they want.
> 
> Hmm, is that so bad though?  I'm not sure I see what problem it could
> cause.  Do people actually want and rely on the indefinite waiting?

They can rely on more than 5 or 10 sec of waiting, yes.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Lars Ingebrigtsen]

Stefan Kangas <stefankangas@gmail.com> writes:

> Hmm, is that so bad though?  I'm not sure I see what problem it could
> cause.  Do people actually want and rely on the indefinite waiting?

If you're controlling Emacs programmatically, you don't want commands to
semi-randomly fail just because the Emacs is busy with the previous
command.

> I often do things like
>
>    alias emacsclient="emacsclient --timeout 10"
>
> so I think such a flag would still be useful.

It could be useful, but I think the number of people that would do
something like this is minuscule, so it's marginal.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Stefan Kangas]

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

tags 50849 + patch
thanks

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I don't think a --timeout parameter will be useful here, though --
> nobody is going to type "emacsclient --timeout 10 -nw".
>
> So I think an informational message (like previously mentioned) is the
> best we can do here.

How about the attached patch?  It adds an informational message after 30
seconds by default.  With a "--timeout N" flag, we instead display a
message and exit (after N seconds).

Note that:

1. I didn't yet add documentation.
2. I've only tested the patch on GNU/Linux (*not* on MS-Windows).
3. To simulate a timeout for testing, you could try this:

diff --git a/lisp/server.el b/lisp/server.el
index 3caa335c4e..5dc220d0b3 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1113,8 +1113,8 @@ server-process-filter
       (progn
 	(server-add-client proc)
 	;; Send our pid
-	(server-send-string proc (concat "-emacs-pid "
-					 (number-to-string (emacs-pid)) "\n"))
+        ;; (server-send-string proc (concat "-emacs-pid "
+        ;; 				 (number-to-string (emacs-pid)) "\n"))
 	(if (not (string-match "\n" string))
             ;; Save for later any partial line that remains.
             (when (> (length string) 0)

[-- Attachment #2: 0001-Add-new-timeout-flag-to-emacsclient.patch --]
[-- Type: text/x-diff, Size: 4930 bytes --]

From 7e418860ad2a7cd5d99737a4a7d187cf0f3d686c Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Mon, 5 Sep 2022 15:50:20 +0200
Subject: [PATCH] Add new --timeout flag to emacsclient

* lib-src/emacsclient.c (DEFAULT_TIMEOUT): New constant.
(timeout): New static variable.
(longopts, shortopts, decode_options, print_help_and_exit): Add new
flag --timeout.
(set_socket_timeout, check_socket_timeout): New helper functions.
(main): Display a status message or exit after Emacs has not responded
for a while, depending on above new --timeout flag.  (Bug#50849)
---
 lib-src/emacsclient.c | 70 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 67 insertions(+), 3 deletions(-)

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index 73c8e45a86..7bc8fc3d4d 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -64,6 +64,8 @@ Copyright (C) 1986-1987, 1994, 1999-2022 Free Software Foundation, Inc.
 
 # define egetenv(VAR) getenv (VAR)
 
+# define DEFAULT_TIMEOUT (30)
+
 #endif /* !WINDOWSNT */
 
 #include <ctype.h>
@@ -144,6 +146,9 @@ Copyright (C) 1986-1987, 1994, 1999-2022 Free Software Foundation, Inc.
 /* If non-NULL, the filename of the authentication file.  */
 static char const *server_file;
 
+/* If non-NULL, seconds to wait before timing out.  */
+static uintmax_t timeout;
+
 /* If non-NULL, the tramp prefix emacs must use to find the files.  */
 static char const *tramp_prefix;
 
@@ -178,6 +183,7 @@ Copyright (C) 1986-1987, 1994, 1999-2022 Free Software Foundation, Inc.
   { "server-file",	required_argument, NULL, 'f' },
   { "display",	required_argument, NULL, 'd' },
   { "parent-id", required_argument, NULL, 'p' },
+  { "timeout",	required_argument, NULL, 'w' },
   { "tramp",	required_argument, NULL, 'T' },
   { 0, 0, 0, 0 }
 };
@@ -185,7 +191,7 @@ Copyright (C) 1986-1987, 1994, 1999-2022 Free Software Foundation, Inc.
 /* Short options, in the same order as the corresponding long options.
    There is no '-p' short option.  */
 static char const shortopts[] =
-  "nqueHVtca:F:"
+  "nqueHVtcaw:F:"
 #ifdef SOCKETS_IN_FILE_SYSTEM
   "s:"
 #endif
@@ -496,7 +502,7 @@ decode_options (int argc, char **argv)
       int opt = getopt_long_only (argc, argv, shortopts, longopts, NULL);
       if (opt < 0)
 	break;
-
+      char* endptr;
       switch (opt)
 	{
 	case 0:
@@ -530,6 +536,17 @@ decode_options (int argc, char **argv)
 	  nowait = true;
 	  break;
 
+	case 'w':
+	  timeout = strtoumax (optarg, &endptr, 10);
+	  if (timeout <= 0 ||
+	      ((timeout == INTMAX_MAX || timeout == INTMAX_MIN)
+	       && errno == ERANGE))
+	    {
+	      fprintf (stderr, "Invalid timeout: \"%s\"\n", optarg);
+	      exit (EXIT_FAILURE);
+	    }
+	  break;
+
 	case 'e':
 	  eval = true;
 	  break;
@@ -671,6 +688,7 @@ print_help_and_exit (void)
 			Set the parameters of a new frame\n\
 -e, --eval    		Evaluate the FILE arguments as ELisp expressions\n\
 -n, --no-wait		Don't wait for the server to return\n\
+-w, --timeout		Seconds to wait before timing out\n\
 -q, --quiet		Don't display messages on success\n\
 -u, --suppress-output   Don't display return values from the server\n\
 -d DISPLAY, --display=DISPLAY\n\
@@ -1870,6 +1888,33 @@ start_daemon_and_retry_set_socket (void)
   return emacs_socket;
 }
 
+static void
+set_socket_timeout (HSOCKET socket, int seconds)
+{
+#ifndef WINDOWSNT
+  struct timeval timeout;
+  timeout.tv_sec = seconds;
+  timeout.tv_usec = 0;
+  setsockopt (socket, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof timeout);
+#else
+  DWORD timeout = seconds * 1000;
+  setsockopt (socket, SOL_SOCKET, SO_RCVTIMEO, (char*) &timeout, sizeof timeout);
+#endif
+}
+
+static bool
+check_socket_timeout (int rl)
+{
+#ifndef WINDOWSNT
+  return (rl == -1)
+    && (errno == EAGAIN)
+    && (errno == EWOULDBLOCK);
+#else
+  return (rl == SOCKET_ERROR)
+    && (WSAGetLastError() != WSAETIMEDOUT);
+#endif
+}
+
 int
 main (int argc, char **argv)
 {
@@ -2086,15 +2131,34 @@ main (int argc, char **argv)
     }
   fflush (stdout);
 
+  set_socket_timeout (emacs_socket, timeout > 0 ? timeout : DEFAULT_TIMEOUT);
   /* Now, wait for an answer and print any messages.  */
   while (exit_status == EXIT_SUCCESS)
     {
+      bool retry = true;
+      bool msg_showed = false;
       do
 	{
 	  act_on_signals (emacs_socket);
 	  rl = recv (emacs_socket, string, BUFSIZ, 0);
+	  retry = check_socket_timeout (rl);
+	  if (retry)
+	    {
+	      if (timeout > 0)
+		{
+		  /* Don't retry if we were given a --timeout flag.  */
+		  fprintf (stderr, "\nServer not responding; timed out after %lu seconds",
+			   timeout);
+		  retry = false;
+		}
+	      else if (!msg_showed)
+		{
+		  msg_showed = true;
+		  fprintf (stderr, "\nServer not responding; use Ctrl+C to break");
+		}
+	    }
 	}
-      while (rl < 0 && errno == EINTR);
+      while ((rl < 0 && errno == EINTR) || retry);
 
       if (rl <= 0)
         break;
-- 
2.30.2

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Lars Ingebrigtsen]

Stefan Kangas <stefankangas@gmail.com> writes:

> How about the attached patch?  It adds an informational message after 30
> seconds by default.  With a "--timeout N" flag, we instead display a
> message and exit (after N seconds).

I haven't tried it, but it looks good to me, so feel free to push it
(after updating the documentation, too).

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Stefan Kangas]

close 50849 29.1
thanks

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I haven't tried it, but it looks good to me, so feel free to push it
> (after updating the documentation, too).

Thanks, pushed (commit 6a19f2a024).

If someone could check if this works on MS-Windows, that would be
appreciated.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Mon, 5 Sep 2022 20:19:54 -0400
> Cc: Eli Zaretskii <eliz@gnu.org>, 50849@debbugs.gnu.org, bugs@gnu.support, 
> 	Visuwesh <visuweshm@gmail.com>
> 
> close 50849 29.1
> thanks
> 
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
> > I haven't tried it, but it looks good to me, so feel free to push it
> > (after updating the documentation, too).
> 
> Thanks, pushed (commit 6a19f2a024).
> 
> If someone could check if this works on MS-Windows, that would be
> appreciated.

How would one test it, please?  Do you have any recipe?

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Visuwesh]

[செவ்வாய் செப்டம்பர் 06, 2022] Eli Zaretskii wrote:

>> If someone could check if this works on MS-Windows, that would be
>> appreciated.
>
> How would one test it, please?  Do you have any recipe?

Here's what I did to test it,

    % src/emacs -Q --daemon
    Starting Emacs daemon.
    % emacsclient --eval '(while t)' &
    [1] 2700
    % emacsclient -c --timeout=2
    Waiting for Emacs...
    Server not responding; timed out after 2 seconds
    % emacsclient -c
    Waiting for Emacs...
    Server not responding; use Ctrl+C to break  C-c C-c

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Robert Pluim]

>>>>> On Mon, 5 Sep 2022 20:19:54 -0400, Stefan Kangas <stefankangas@gmail.com> said:

    Stefan> close 50849 29.1
    Stefan> thanks

    Stefan> Lars Ingebrigtsen <larsi@gnus.org> writes:

    >> I haven't tried it, but it looks good to me, so feel free to push it
    >> (after updating the documentation, too).

    Stefan> Thanks, pushed (commit 6a19f2a024).

Aagh, that was a bit quick. What about if I want to try forever, but
be informed every <n> seconds that emacs hasnʼt responded yet? (I
thought that was in the original proposal?)

Robert
-- 

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Lars Ingebrigtsen]

Robert Pluim <rpluim@gmail.com> writes:

> Aagh, that was a bit quick. What about if I want to try forever, but
> be informed every <n> seconds that emacs hasnʼt responded yet? (I
> thought that was in the original proposal?)

I think one single warning is what the vast majority of people would
want to see, so adding something to twiddle this doesn't seem useful.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Visuwesh <visuweshm@gmail.com>
> Cc: Stefan Kangas <stefankangas@gmail.com>,  larsi@gnus.org,
>   bugs@gnu.support,  50849@debbugs.gnu.org
> Date: Tue, 06 Sep 2022 09:03:24 +0530
> 
> [செவ்வாய் செப்டம்பர் 06, 2022] Eli Zaretskii wrote:
> 
> >> If someone could check if this works on MS-Windows, that would be
> >> appreciated.
> >
> > How would one test it, please?  Do you have any recipe?
> 
> Here's what I did to test it,
> 
>     % src/emacs -Q --daemon
>     Starting Emacs daemon.
>     % emacsclient --eval '(while t)' &
>     [1] 2700
>     % emacsclient -c --timeout=2
>     Waiting for Emacs...
>     Server not responding; timed out after 2 seconds
>     % emacsclient -c
>     Waiting for Emacs...
>     Server not responding; use Ctrl+C to break  C-c C-c

Thanks.

In fact, one can test this much more easily:

  emacs -Q &
  M-x server-start RET

  emacsclient -w N SOME-FILE
  Waiting for Emacs...

Observe that SOME-FILE is displayed by the server in a client frame.

Now wait for N seconds without doing anything and observe:

  Server not responding; use Ctrl+C to break

IOW, emacsclient _always_ reports a timeout after N seconds, even
though the server did act upon the client request.

Looking at the code, I don't understand how this was supposed to
work.  After sending the request to the server, we call recv in a
loop, waiting for a response.  But in a normal session, the server
will only respond when the user is done with editing the file, which
could be after a very long time.  So this _must_ time out.

I think to make this work, the client-server protocol should be
changed so that the server responds with some kind of positive
response right after it receives the initial request.  And that would
make the protocol backward-incompatible, unfortunately.

Alternatively, perhaps some different change in set_socket and its
subroutines could be used?  Isn't that where we get stuck if the
server is busy?  But I'm really out of my depth here, so would socket
experts please chime in?

And another comment: the documentation says that the default time out
is zero, i.e. no timeout, but that's not true: the default timeout is
actually 30 sec.  So if someone uses the client of the master branch,
it will now always terminate due to timeout after 30 sec...

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Robert Pluim]

>>>>> On Tue, 06 Sep 2022 15:22:27 +0300, Eli Zaretskii <eliz@gnu.org> said:

    Eli> Looking at the code, I don't understand how this was supposed to
    Eli> work.  After sending the request to the server, we call recv in a
    Eli> loop, waiting for a response.  But in a normal session, the server
    Eli> will only respond when the user is done with editing the file, which
    Eli> could be after a very long time.  So this _must_ time out.

Doesnʼt the server send its pid to the client? Youʼd want to
recv+timeout just for that initial response, no?

    Eli> I think to make this work, the client-server protocol should be
    Eli> changed so that the server responds with some kind of positive
    Eli> response right after it receives the initial request.  And that would
    Eli> make the protocol backward-incompatible, unfortunately.

Both emacsclient.c and server.el error out on unknown commands, so
thatʼs inevitable.

Robert
-- 

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: stefankangas@gmail.com,  Visuwesh <visuweshm@gmail.com>,
>   larsi@gnus.org,  bugs@gnu.support,  50849@debbugs.gnu.org
> Date: Tue, 06 Sep 2022 16:02:01 +0200
> 
> >>>>> On Tue, 06 Sep 2022 15:22:27 +0300, Eli Zaretskii <eliz@gnu.org> said:
> 
>     Eli> Looking at the code, I don't understand how this was supposed to
>     Eli> work.  After sending the request to the server, we call recv in a
>     Eli> loop, waiting for a response.  But in a normal session, the server
>     Eli> will only respond when the user is done with editing the file, which
>     Eli> could be after a very long time.  So this _must_ time out.
> 
> Doesnʼt the server send its pid to the client? Youʼd want to
> recv+timeout just for that initial response, no?

Maybe, I don't know.  If that's always so, then yes, the code should
be restructured to time out only on that single response.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Robert Pluim]

>>>>> On Tue, 06 Sep 2022 17:12:15 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: stefankangas@gmail.com,  Visuwesh <visuweshm@gmail.com>,
    >> larsi@gnus.org,  bugs@gnu.support,  50849@debbugs.gnu.org
    >> Date: Tue, 06 Sep 2022 16:02:01 +0200
    >> 
    >> >>>>> On Tue, 06 Sep 2022 15:22:27 +0300, Eli Zaretskii <eliz@gnu.org> said:
    >> 
    Eli> Looking at the code, I don't understand how this was supposed to
    Eli> work.  After sending the request to the server, we call recv in a
    Eli> loop, waiting for a response.  But in a normal session, the server
    Eli> will only respond when the user is done with editing the file, which
    Eli> could be after a very long time.  So this _must_ time out.
    >> 
    >> Doesnʼt the server send its pid to the client? Youʼd want to
    >> recv+timeout just for that initial response, no?

    Eli> Maybe, I don't know.  If that's always so, then yes, the code should
    Eli> be restructured to time out only on that single response.

server-process-filter unconditionally does:

	(server-send-string proc (concat "-emacs-pid "
					 (number-to-string (emacs-pid)) "\n"))

Robert
-- 

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> Now wait for N seconds without doing anything and observe:
>
>   Server not responding; use Ctrl+C to break

This should now be fixed on master.

> After sending the request to the server, we call recv in a
> loop, waiting for a response.  But in a normal session, the server
> will only respond when the user is done with editing the file, which
> could be after a very long time.  So this _must_ time out.

No, server.el will always send "-emacs-pid 1211057".  You can see this
in the " *server*" buffer (set `server-log' to t first).

> And another comment: the documentation says that the default time out
> is zero, i.e. no timeout, but that's not true: the default timeout is
> actually 30 sec.  So if someone uses the client of the master branch,
> it will now always terminate due to timeout after 30 sec...

I can't reproduce this.  Do you have a recipe?

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Tue, 6 Sep 2022 21:05:50 -0400
> Cc: larsi@gnus.org, bugs@gnu.support, 50849@debbugs.gnu.org
> 
> > And another comment: the documentation says that the default time out
> > is zero, i.e. no timeout, but that's not true: the default timeout is
> > actually 30 sec.  So if someone uses the client of the master branch,
> > it will now always terminate due to timeout after 30 sec...
> 
> I can't reproduce this.  Do you have a recipe?

Do I need a recipe?  The source code says:

  #define DEFAULT_TIMEOUT (30)
  [...]
    set_socket_timeout (emacs_socket, timeout > 0 ? timeout : DEFAULT_TIMEOUT);

That's self-explanatory, isn't it?

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> Do I need a recipe?  The source code says:
>
>   #define DEFAULT_TIMEOUT (30)
>   [...]
>     set_socket_timeout (emacs_socket, timeout > 0 ? timeout : DEFAULT_TIMEOUT);
>
> That's self-explanatory, isn't it?

You wrote that "if someone uses the client of the master branch, it will
now always terminate due to timeout after 30 sec", but I don't see that
on current master.  So I'm asking if we are not seeing the same
behaviour, and if so, why that is.

When I do this:

    ./lib/emacsclient foo.txt

and wait for more than 30 seconds, emacsclient doesn't exit.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Robert Pluim]

>>>>> On Wed, 7 Sep 2022 04:18:17 -0400, Stefan Kangas <stefankangas@gmail.com> said:

    Stefan> Eli Zaretskii <eliz@gnu.org> writes:
    >> Do I need a recipe?  The source code says:
    >> 
    >> #define DEFAULT_TIMEOUT (30)
    >> [...]
    >> set_socket_timeout (emacs_socket, timeout > 0 ? timeout : DEFAULT_TIMEOUT);
    >> 
    >> That's self-explanatory, isn't it?

    Stefan> You wrote that "if someone uses the client of the master branch, it will
    Stefan> now always terminate due to timeout after 30 sec", but I don't see that
    Stefan> on current master.  So I'm asking if we are not seeing the same
    Stefan> behaviour, and if so, why that is.

    Stefan> When I do this:

    Stefan>     ./lib/emacsclient foo.txt

    Stefan> and wait for more than 30 seconds, emacsclient doesn't exit.

...because the code on master now just retries the recv instead of
exiting. So in normal operation without a timeout argument, the recv
will timeout after 30 seconds, and emacsclient will go back to
recv. Before the timeout changes it would wait in recv forever.

Thatʼs a behaviour change. I guess we could change emacsclient to set
the timeout back to 0 in that case, but Iʼm not sure itʼs worth it.

Robert
-- 

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  larsi@gnus.org,  50849@debbugs.gnu.org,
>   bugs@gnu.support,  visuweshm@gmail.com
> Date: Wed, 07 Sep 2022 12:34:35 +0200
> 
> >>>>> On Wed, 7 Sep 2022 04:18:17 -0400, Stefan Kangas <stefankangas@gmail.com> said:
> 
>     Stefan> You wrote that "if someone uses the client of the master branch, it will
>     Stefan> now always terminate due to timeout after 30 sec", but I don't see that
>     Stefan> on current master.  So I'm asking if we are not seeing the same
>     Stefan> behaviour, and if so, why that is.
> 
>     Stefan> When I do this:
> 
>     Stefan>     ./lib/emacsclient foo.txt
> 
>     Stefan> and wait for more than 30 seconds, emacsclient doesn't exit.
> 
> ...because the code on master now just retries the recv instead of
> exiting. So in normal operation without a timeout argument, the recv
> will timeout after 30 seconds, and emacsclient will go back to
> recv. Before the timeout changes it would wait in recv forever.
> 
> Thatʼs a behaviour change. I guess we could change emacsclient to set
> the timeout back to 0 in that case, but Iʼm not sure itʼs worth it.

Exactly.  I actually don't understand why we need to call setsockopt
(without checking errors) and then complicate our lives with no less
than 3 tricky-named flags ('retry' is not really what its name says,
msg_showed is initialized with a non-fixed value, etc.) when the
timeout was not given.  Why not just avoid setting the timeout in that
case?

And in any case, saying that the default timeout is zero is simply
misleading.  We should either say that "by default emacscilent will
wait indefinitely" or modify DEFAULT_TIMEOUT to zero.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> I actually don't understand why we need to call setsockopt

We need to call setsockopt to set the timeout.

Based on section 13.2 in UNIX Network Programming, I guess we could call
alarm(2) and set up a SIGALRM handler instead.  (But that seems to have
been mentioned there mostly in case you need to use an implementation
that doesn't support SO_RCVTIMEO, so I'm not sure it's a good idea.)

> (without checking errors)

Adding error handling is easy enough, but what do you think we must do
differently if the call to setsockopt fails?  IIUC, the recv call will
just not timeout in that case, and things will be as if we hadn't tried
to set any timeout at all.

> and then complicate our lives with no less
> than 3 tricky-named flags ('retry' is not really what its name says,
> msg_showed is initialized with a non-fixed value, etc.) when the
> timeout was not given.

What would you name these flags instead?  And what does "etc." above
mean?

Regarding msg_showed, I think you're right that it should be
initialized to a constant value instead.

> Why not just avoid setting the timeout in that case?

Because we want to give the informational message "Server not
responding; use Ctrl+C to break".  If we don't want that message, we
don't need to set a timeout in that case.

> And in any case, saying that the default timeout is zero is simply
> misleading.  We should either say that "by default emacscilent will
> wait indefinitely" or modify DEFAULT_TIMEOUT to zero.

I think it makes sense to change the documentation as you suggest.

(Note that DEFAULT_TIMEOUT really only has to do with the time to wait
before printing an informational message, in the case when we did not
get a --timeout flag.  Maybe it could get a better name.)

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Wed, 7 Sep 2022 18:47:03 -0700
> Cc: larsi@gnus.org, 50849@debbugs.gnu.org, bugs@gnu.support, 
> 	visuweshm@gmail.com
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I actually don't understand why we need to call setsockopt
> 
> We need to call setsockopt to set the timeout.

When the timeout wasn't given by the user, I don't think it's our
business to second-guess what the user wants.

Setting a timeout in all cases is at least an incompatible behavior
change.  For example, imagine a use case where the delay is indeed
justified, and the user has no problems having emacsclient to wait for
more than 30 sec.  With the current code on master, we will output
messages that we previously didn't, and that could potentially break
someone's scripts.  Moreover, there's no way for users in such cases
to get back the old behavior, none whatsoever.  I don't see how can we
justify such an incompatible change.  The imaginary case I described
is probably rare at best, but it's still a legitimate one, and we had
better avoided breaking it, unless we have a very good reason.

> Based on section 13.2 in UNIX Network Programming, I guess we could call
> alarm(2) and set up a SIGALRM handler instead.

SIGALRM is non-portable, so it's a non-starter.

> > (without checking errors)
> 
> Adding error handling is easy enough, but what do you think we must do
> differently if the call to setsockopt fails?

Display an error message?

> IIUC, the recv call will just not timeout in that case, and things
> will be as if we hadn't tried to set any timeout at all.

Exactly.  But it means the user asked us to do something, and we
didn't.  The textbook behavior in such cases is to let the user know
that the contract was broken.

> > and then complicate our lives with no less
> > than 3 tricky-named flags ('retry' is not really what its name says,
> > msg_showed is initialized with a non-fixed value, etc.) when the
> > timeout was not given.
> 
> What would you name these flags instead?

Something that really tells us what they signify and how they are
used.  When there are 3 flags whose different combinations mean
different things, the code can be difficult to follow, understand, and
modify safely.

> And what does "etc." above mean?

That I could continue telling why the current flag-based
implementation is hard to read and understand.  At the very least, we
should have comments there explaining the logic.

> > Why not just avoid setting the timeout in that case?
> 
> Because we want to give the informational message "Server not
> responding; use Ctrl+C to break".  If we don't want that message, we
> don't need to set a timeout in that case.

See above: when the user doesn't use --timeout, he/she doesn't
necessarily want emacsclient to brag about the server not responding.

> > And in any case, saying that the default timeout is zero is simply
> > misleading.  We should either say that "by default emacscilent will
> > wait indefinitely" or modify DEFAULT_TIMEOUT to zero.
> 
> I think it makes sense to change the documentation as you suggest.

Then please do.  We currently don't describe the default behavior
correctly.  (Of course, if you eventually agree to change the code so
that no timeout is set on the socket, we just need to say that by
default there's no timeout.)

> (Note that DEFAULT_TIMEOUT really only has to do with the time to wait
> before printing an informational message, in the case when we did not
> get a --timeout flag.  Maybe it could get a better name.)

Either a better name (but what would be such a name?), or better
comments and documentation telling what it does.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> With the current code on master, we will output
> messages that we previously didn't, and that could potentially break
> someone's scripts.

That is a potential problem, but I think the advantage (not seemingly
hanging without output) is worth it -- especially if the message is on
stderr.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Stefan Kangas <stefankangas@gmail.com>,  rpluim@gmail.com,
>   50849@debbugs.gnu.org,  bugs@gnu.support,  visuweshm@gmail.com
> Date: Thu, 08 Sep 2022 14:07:38 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > With the current code on master, we will output
> > messages that we previously didn't, and that could potentially break
> > someone's scripts.
> 
> That is a potential problem, but I think the advantage (not seemingly
> hanging without output) is worth it -- especially if the message is on
> stderr.

For you, that advantage overrides the potential incompatibility, but
for someone else the balance could be the other way around.

Why not leave this as an opt-in feature, for those who really need it?
After all, shell aliases are available to save extra typing.  So
what's the downside?

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Why not leave this as an opt-in feature, for those who really need it?
> After all, shell aliases are available to save extra typing.  So
> what's the downside?

As I said earlier, nobody is going to write emacsclient
--give-me-feedback or whatever, so adding such an option is of little
value.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: stefankangas@gmail.com,  rpluim@gmail.com,  50849@debbugs.gnu.org,
>   bugs@gnu.support,  visuweshm@gmail.com
> Date: Thu, 08 Sep 2022 15:46:15 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Why not leave this as an opt-in feature, for those who really need it?
> > After all, shell aliases are available to save extra typing.  So
> > what's the downside?
> 
> As I said earlier, nobody is going to write emacsclient
> --give-me-feedback or whatever, so adding such an option is of little
> value.

??? We already added --timeout, so what do you mean by that?

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> ??? We already added --timeout, so what do you mean by that?

I don't expect anybody to type --timeout either, but I can see that it
might be useful when scripting, for instance.  --give-me-feedback, on
the other hand, would never be used.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: stefankangas@gmail.com,  rpluim@gmail.com,  50849@debbugs.gnu.org,
>   bugs@gnu.support,  visuweshm@gmail.com
> Date: Thu, 08 Sep 2022 16:11:32 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > ??? We already added --timeout, so what do you mean by that?
> 
> I don't expect anybody to type --timeout either, but I can see that it
> might be useful when scripting, for instance.  --give-me-feedback, on
> the other hand, would never be used.

I think there's some misunderstanding: I wasn't taking about anything
called --give-me-feedback.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Robert Pluim]

>>>>> On Thu, 08 Sep 2022 17:15:09 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Lars Ingebrigtsen <larsi@gnus.org>
    >> Cc: stefankangas@gmail.com,  rpluim@gmail.com,  50849@debbugs.gnu.org,
    >> bugs@gnu.support,  visuweshm@gmail.com
    >> Date: Thu, 08 Sep 2022 16:11:32 +0200
    >> 
    >> Eli Zaretskii <eliz@gnu.org> writes:
    >> 
    >> > ??? We already added --timeout, so what do you mean by that?
    >> 
    >> I don't expect anybody to type --timeout either, but I can see that it
    >> might be useful when scripting, for instance.  --give-me-feedback, on
    >> the other hand, would never be used.

    Eli> I think there's some misunderstanding: I wasn't taking about anything
    Eli> called --give-me-feedback.

So if we set DEFAULT_TIMEOUT to 0, people who do nothing get no
messages, and people who do --timeout get messages. I think that
satisfies Eliʼs concern about compatibility (Iʼd even go so far as not
setting the timeout at all if itʼs not requested. Why yes, I am
paranoid about changing old code).

Robert
-- 

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  stefankangas@gmail.com,
>   50849@debbugs.gnu.org,  bugs@gnu.support,  visuweshm@gmail.com
> Date: Thu, 08 Sep 2022 16:19:40 +0200
> 
> >>>>> On Thu, 08 Sep 2022 17:15:09 +0300, Eli Zaretskii <eliz@gnu.org> said:
> 
>     >> From: Lars Ingebrigtsen <larsi@gnus.org>
>     >> Cc: stefankangas@gmail.com,  rpluim@gmail.com,  50849@debbugs.gnu.org,
>     >> bugs@gnu.support,  visuweshm@gmail.com
>     >> Date: Thu, 08 Sep 2022 16:11:32 +0200
>     >> 
>     >> Eli Zaretskii <eliz@gnu.org> writes:
>     >> 
>     >> > ??? We already added --timeout, so what do you mean by that?
>     >> 
>     >> I don't expect anybody to type --timeout either, but I can see that it
>     >> might be useful when scripting, for instance.  --give-me-feedback, on
>     >> the other hand, would never be used.
> 
>     Eli> I think there's some misunderstanding: I wasn't taking about anything
>     Eli> called --give-me-feedback.
> 
> So if we set DEFAULT_TIMEOUT to 0, people who do nothing get no
> messages, and people who do --timeout get messages. I think that
> satisfies Eliʼs concern about compatibility

Yes, I think so.

> (Iʼd even go so far as not setting the timeout at all if itʼs not
> requested. Why yes, I am paranoid about changing old code).

I agree.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Robert Pluim]

>>>>> On Thu, 08 Sep 2022 19:02:50 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> So if we set DEFAULT_TIMEOUT to 0, people who do nothing get no
    >> messages, and people who do --timeout get messages. I think that
    >> satisfies Eliʼs concern about compatibility

    Eli> Yes, I think so.

    >> (Iʼd even go so far as not setting the timeout at all if itʼs not
    >> requested. Why yes, I am paranoid about changing old code).

    Eli> I agree.

So this simplifies the code considerably, and in fact removes the
whole retry thing completely.

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index 49d90a989f..b9ade602e4 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -66,8 +66,6 @@ Copyright (C) 1986-2022 Free Software Foundation, Inc.
 
 #endif /* !WINDOWSNT */
 
-#define DEFAULT_TIMEOUT (30)
-
 #include <ctype.h>
 #include <errno.h>
 #include <getopt.h>
@@ -2137,35 +2135,24 @@ main (int argc, char **argv)
     }
   fflush (stdout);
 
-  set_socket_timeout (emacs_socket, timeout > 0 ? timeout : DEFAULT_TIMEOUT);
+  if (timeout > 0)
+      set_socket_timeout (emacs_socket, timeout);
   bool saw_response = false;
+
   /* Now, wait for an answer and print any messages.  */
   while (exit_status == EXIT_SUCCESS)
     {
-      bool retry = true;
-      bool msg_showed = quiet;
       do
 	{
 	  act_on_signals (emacs_socket);
 	  rl = recv (emacs_socket, string, BUFSIZ, 0);
-	  retry = check_socket_timeout (rl);
-	  if (retry && !saw_response)
-	    {
-	      if (timeout > 0)
-		{
-		  /* Don't retry if we were given a --timeout flag.  */
-		  fprintf (stderr, "\nServer not responding; timed out after %lu seconds",
-			   timeout);
-		  retry = false;
-		}
-	      else if (!msg_showed)
-		{
-		  msg_showed = true;
-		  fprintf (stderr, "\nServer not responding; use Ctrl+C to break");
-		}
-	    }
+	  if (timeout > 0
+	      && check_socket_timeout (rl)
+	      && !saw_response)
+	    fprintf (stderr, "\nServer not responding; timed out after %lu seconds",
+		     timeout);
 	}
-      while ((rl < 0 && errno == EINTR) || retry);
+      while (rl < 0 && errno == EINTR);
 
       if (rl <= 0)
         break;

Robert
-- 

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Stefan Kangas]

Robert Pluim <rpluim@gmail.com> writes:

> So this simplifies the code considerably, and in fact removes the
> whole retry thing completely.

We could do that, yes.  It looks good to me.

I had a different idea in mind though, which lets us keep the diagnostic
message, but only if given a --verbose flag.  We could extend --verbose
to include more diagnostic output in the future.  Please see the
attached patch.

I'm happy with either your patch or mine, depending on what people
prefer.

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index 88800b9b2e..cd3069d7a8 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -66,7 +66,9 @@ Copyright (C) 1986-2022 Free Software Foundation, Inc.

 #endif /* !WINDOWSNT */

-#define DEFAULT_TIMEOUT (30)
+/* Seconds to wait before warning that the server hasn't responded.
+   Only applicable with --verbose and without any --timeout flag.  */
+#define DEFAULT_WARN_AFTER (30)

 #include <ctype.h>
 #include <errno.h>
@@ -110,6 +112,9 @@ #define DEFAULT_TIMEOUT (30)
 /* True means don't print messages for successful operations.  --quiet.  */
 static bool quiet;

+/* True means print verbose messages.  --verbose.  */
+static bool verbose;
+
 /* True means don't print values returned from emacs. --suppress-output.  */
 static bool suppress_output;

@@ -166,6 +171,7 @@ #define DEFAULT_TIMEOUT (30)
 static struct option const longopts[] =
 {
   { "no-wait",	no_argument,	   NULL, 'n' },
+  { "verbose",	no_argument,	   NULL, 'v' },
   { "quiet",	no_argument,	   NULL, 'q' },
   { "suppress-output", no_argument, NULL, 'u' },
   { "eval",	no_argument,	   NULL, 'e' },
@@ -191,7 +197,7 @@ #define DEFAULT_TIMEOUT (30)
 /* Short options, in the same order as the corresponding long options.
    There is no '-p' short option.  */
 static char const shortopts[] =
-  "nqueHVtca:F:w:"
+  "nqueHVtcva:F:w:"
 #ifdef SOCKETS_IN_FILE_SYSTEM
   "s:"
 #endif
@@ -488,6 +494,23 @@ message (bool is_error, const char *format, ...)
   va_end (args);
 }

+/* Print message only if we got the --verbose flag.  */
+static void print_verbose (const char *, ...) ATTRIBUTE_FORMAT_PRINTF (1, 2);
+static void
+print_verbose (const char *format, ...)
+{
+  va_list args;
+  va_start (args, format);
+
+  if (verbose)
+    {
+      vfprintf (stderr, format, args);
+      fflush (stderr);
+    }
+
+  va_end (args);
+}
+
 /* Decode the options from argv and argc.
    The global variable 'optind' will say how many arguments we used up.  */

@@ -556,6 +579,10 @@ decode_options (int argc, char **argv)
 	  quiet = true;
 	  break;

+	case 'v':
+	  verbose = true;
+	  break;
+
 	case 'u':
 	  suppress_output = true;
 	  break;
@@ -2144,40 +2171,49 @@ main (int argc, char **argv)
     }
   fflush (stdout);

-  set_socket_timeout (emacs_socket, timeout > 0 ? timeout : DEFAULT_TIMEOUT);
-  bool saw_response = false;
+  bool should_timeout = timeout > 0 || verbose;
   /* Now, wait for an answer and print any messages.  */
   while (exit_status == EXIT_SUCCESS)
     {
-      bool retry = true;
-      bool msg_showed = quiet;
+      if (should_timeout)
+	set_socket_timeout (emacs_socket,
+			    timeout > 0 ? timeout : DEFAULT_WARN_AFTER);
       do
 	{
+	retry_recv:
 	  act_on_signals (emacs_socket);
 	  rl = recv (emacs_socket, string, BUFSIZ, 0);
-	  retry = check_socket_timeout (rl);
-	  if (retry && !saw_response)
+
+	  /* Handle --timeout and --verbose.  */
+	  if (should_timeout
+	      /* Just retry if we got EINTR; it's not a timeout.  */
+	      && errno != EINTR)
 	    {
-	      if (timeout > 0)
-		{
-		  /* Don't retry if we were given a --timeout flag.  */
-		  fprintf (stderr, "\nServer not responding; timed out after %lu seconds",
-			   timeout);
-		  retry = false;
-		}
-	      else if (!msg_showed)
+	      should_timeout = false;
+	      set_socket_timeout (emacs_socket, 0);
+
+	      if (check_socket_timeout (rl))
 		{
-		  msg_showed = true;
-		  fprintf (stderr, "\nServer not responding; use Ctrl+C to break");
+		  if (timeout > 0)
+		    {
+		      fprintf (stderr,
+			       "\nServer not responding; timed out after %lu seconds",
+			       timeout);
+		      break;
+		    }
+		  else /* if (verbose) */
+		    {
+		      print_verbose ("\nServer not responding; use Ctrl+C to break");
+		      goto retry_recv;
+		    }
 		}
 	    }
 	}
-      while ((rl < 0 && errno == EINTR) || retry);
+      while (rl < 0 && errno == EINTR);

       if (rl <= 0)
         break;

-      saw_response = true;
       string[rl] = '\0';

       /* Loop over all NL-terminated messages.  */

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Robert Pluim]

>>>>> On Fri, 9 Sep 2022 05:29:17 -0400, Stefan Kangas <stefankangas@gmail.com> said:

    Stefan> Robert Pluim <rpluim@gmail.com> writes:
    >> So this simplifies the code considerably, and in fact removes the
    >> whole retry thing completely.

    Stefan> We could do that, yes.  It looks good to me.

    Stefan> I had a different idea in mind though, which lets us keep the diagnostic
    Stefan> message, but only if given a --verbose flag.  We could extend --verbose
    Stefan> to include more diagnostic output in the future.  Please see the
    Stefan> attached patch.

Adding a --verbose flag is fine. Changing the recv behaviour because
the user asked for more debug is a no-no.

    Stefan> I'm happy with either your patch or mine, depending on what people
    Stefan> prefer.

I wonʼt insist on mine, but I really donʼt think yours should go in
as-is.

Robert
-- 

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Stefan Kangas]

Robert Pluim <rpluim@gmail.com> writes:

> Adding a --verbose flag is fine. Changing the recv behaviour because
> the user asked for more debug is a no-no.

I don't see a way around that.

bug#50849: 28.0.50; Proposal for Emacs daemon to signal when being busy

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: larsi@gnus.org,  visuweshm@gmail.com,  stefankangas@gmail.com,
>   bugs@gnu.support,  50849@debbugs.gnu.org
> Date: Fri, 09 Sep 2022 10:47:23 +0200
> 
> >>>>> On Thu, 08 Sep 2022 19:02:50 +0300, Eli Zaretskii <eliz@gnu.org> said:
> 
>     >> So if we set DEFAULT_TIMEOUT to 0, people who do nothing get no
>     >> messages, and people who do --timeout get messages. I think that
>     >> satisfies Eliʼs concern about compatibility
> 
>     Eli> Yes, I think so.
> 
>     >> (Iʼd even go so far as not setting the timeout at all if itʼs not
>     >> requested. Why yes, I am paranoid about changing old code).
> 
>     Eli> I agree.
> 
> So this simplifies the code considerably, and in fact removes the
> whole retry thing completely.

I agree that the code is much easier to follow after these changes.

Thanks.