all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: Kaushal Modi <kaushal.modi@gmail.com>,
	Emacs developers <emacs-devel@gnu.org>
Subject: Re: emacsclient not working on RHEL 6.8 (non-sudo) [master branch]
Date: Thu, 6 Dec 2018 10:52:31 -0800	[thread overview]
Message-ID: <ae98e017-3c69-1cdc-0a0b-7608fdf1ea18@cs.ucla.edu> (raw)
In-Reply-To: <CAFyQvY0g=sL6nRKkbq7ay_40nkuAkPyCq=5chvn93wYnqzPsDg@mail.gmail.com>

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

Thanks, I reproduced the problem and installed the following patches. 
The first should fix the bug. The second should fix some display 
glitches I noticed while fixing the bug, where emacsclient wrote to the 
terminal even though it was started with a trailing '&'.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-emacsclient-hang-when-backgrounded.patch --]
[-- Type: text/x-patch; name="0001-Fix-emacsclient-hang-when-backgrounded.patch", Size: 2474 bytes --]

From 2f985977f691a37a6d45298128b88d0cefcc93a1 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 6 Dec 2018 08:54:00 -0800
Subject: [PATCH 1/2] Fix emacsclient hang when backgrounded
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Kaushal Modi in:
https://lists.gnu.org/r/emacs-devel/2018-12/msg00083.html
The tcdrain call replaced an fdatasync call which had no
effect on the tty, so removing it entirely shouldn’t cause
problems.  The fdatasync call replaced an fsync call which
also had no effect on the tty, and the fsync call seems to be
badly-merged revenant of emacsclient’s old (circa 2004) way of
communicating to and from Emacs via FILE * streams, where
fsync was apparently needed when talking to sockets.
* lib-src/emacsclient.c [!DOS_NT]: Don’t include termios.h.
(flush_stdout): Remove.  All callers removed.
(main): Do not drain the tty after "Waiting for Emacs..."
message.  There should be no need to drain, and draining it
might send us a SIGTTOU.  Do not fflush stdout just before
exiting, as exiting does that for us.
---
 lib-src/emacsclient.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index 7de3665114..653ab955df 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -66,10 +66,6 @@ char *w32_getenv (const char *);
 
 #endif /* !WINDOWSNT */
 
-#ifndef DOS_NT
-# include <termios.h>
-#endif
-
 #include <ctype.h>
 #include <errno.h>
 #include <getopt.h>
@@ -1740,15 +1736,6 @@ start_daemon_and_retry_set_socket (void)
   return emacs_socket;
 }
 
-/* Flush standard output and its underlying file descriptor.  */
-static void
-flush_stdout (HSOCKET emacs_socket)
-{
-  fflush (stdout);
-  while (tcdrain (STDOUT_FILENO) != 0 && errno == EINTR)
-    act_on_signals (emacs_socket);
-}
-
 int
 main (int argc, char **argv)
 {
@@ -1964,7 +1951,7 @@ main (int argc, char **argv)
       printf ("Waiting for Emacs...");
       skiplf = false;
     }
-  flush_stdout (emacs_socket);
+  fflush (stdout);
 
   /* Now, wait for an answer and print any messages.  */
   while (exit_status == EXIT_SUCCESS)
@@ -2067,7 +2054,6 @@ main (int argc, char **argv)
 
   if (!skiplf)
     printf ("\n");
-  flush_stdout (emacs_socket);
 
   if (rl < 0)
     exit_status = EXIT_FAILURE;
-- 
2.19.2


[-- Attachment #3: 0002-emacsclient-avoid-background-chatter.patch --]
[-- Type: text/x-patch, Size: 3111 bytes --]

From 46b810081165fecae5086b71fafdb3eb19c30df5 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 6 Dec 2018 10:46:06 -0800
Subject: [PATCH 2/2] emacsclient: avoid background chatter

* lib-src/emacsclient.c (process_grouping): New function.
(act_on_signals, main): Use it.
(main): Omit "Waiting for Emacs..." and later "\n" messages
if in background, since that messes up the screen.
---
 lib-src/emacsclient.c | 51 ++++++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index 653ab955df..c596fb23ae 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -1095,6 +1095,26 @@ find_tty (const char **tty_type, const char **tty_name, bool noabort)
   return true;
 }
 
+/* Return the process group if in the foreground, the negative of the
+   process group if in the background, and zero if there is no
+   foreground process group for the controlling terminal.
+   Unfortunately, use of this function introduces an unavoidable race,
+   since whether the process is in the foreground or background can
+   change at any time.  */
+
+static pid_t
+process_grouping (void)
+{
+#ifdef SOCKETS_IN_FILE_SYSTEM
+  pid_t tcpgrp = tcgetpgrp (STDOUT_FILENO);
+  if (0 <= tcpgrp)
+    {
+      pid_t pgrp = getpgrp ();
+      return tcpgrp == pgrp ? pgrp : -pgrp;
+    }
+#endif
+  return 0;
+}
 
 #ifdef SOCKETS_IN_FILE_SYSTEM
 
@@ -1253,21 +1273,17 @@ act_on_signals (HSOCKET emacs_socket)
 	    {
 	      got_sigcont = 0;
 	      took_action = true;
-	      pid_t tcpgrp = tcgetpgrp (STDOUT_FILENO);
-	      if (0 <= tcpgrp)
+	      pid_t grouping = process_grouping ();
+	      if (grouping < 0)
 		{
-		  pid_t pgrp = getpgrp ();
-		  if (tcpgrp == pgrp)
-		    {
-		      /* We are in the foreground.  */
-		      send_to_emacs (emacs_socket, "-resume \n");
-		    }
-		  else if (tty)
+		  if (tty)
 		    {
-		      /* We are in the background; cancel the continue.  */
-		      kill (-pgrp, SIGTTIN);
+		      /* Cancel the continue.  */
+		      kill (grouping, SIGTTIN);
 		    }
 		}
+	      else
+		send_to_emacs (emacs_socket, "-resume \n");
 	    }
 
 	  if (got_sigtstp)
@@ -1767,13 +1783,12 @@ main (int argc, char **argv)
       exit (EXIT_FAILURE);
     }
 
-#ifndef WINDOWSNT
+#ifdef SOCKETS_IN_FILE_SYSTEM
   if (tty)
     {
-      pid_t pgrp = getpgrp ();
-      pid_t tcpgrp = tcgetpgrp (STDOUT_FILENO);
-      if (0 <= tcpgrp && tcpgrp != pgrp)
-	kill (-pgrp, SIGTTIN);
+      pid_t grouping = process_grouping ();
+      if (grouping < 0)
+	kill (grouping, SIGTTIN);
     }
 #endif
 
@@ -1946,7 +1961,7 @@ main (int argc, char **argv)
   send_to_emacs (emacs_socket, "\n");
 
   /* Wait for an answer. */
-  if (!eval && !tty && !nowait && !quiet)
+  if (!eval && !tty && !nowait && !quiet && 0 <= process_grouping ())
     {
       printf ("Waiting for Emacs...");
       skiplf = false;
@@ -2052,7 +2067,7 @@ main (int argc, char **argv)
 	}
     }
 
-  if (!skiplf)
+  if (!skiplf && 0 <= process_grouping ())
     printf ("\n");
 
   if (rl < 0)
-- 
2.19.2


  parent reply	other threads:[~2018-12-06 18:52 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-05 16:57 emacsclient not working on RHEL 6.8 (non-sudo) [master branch] Kaushal Modi
2018-12-05 17:36 ` Kaushal Modi
2018-12-05 19:17   ` Kaushal Modi
2018-12-06 18:52 ` Paul Eggert [this message]
2018-12-06 19:29   ` Kaushal Modi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ae98e017-3c69-1cdc-0a0b-7608fdf1ea18@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=emacs-devel@gnu.org \
    --cc=kaushal.modi@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.