bug#13387: emacsclient -t suspends emacs

bug#13387: emacsclient -t suspends emacs

[Dan Nicolaescu]

With the current trunk:

emacs -Q &
M-x server-start RET
and  from the same terminal do:
emacsclient -t

This results in:
[1]  + Suspended (tty output)        emacs -Q

This is a regression, it works just fine in 24.1

bug#13387: emacsclient -t suspends emacs

[Dan Nicolaescu]

Dan Nicolaescu <dann@gnu.org> writes:

> With the current trunk:
>
> emacs -Q &
> M-x server-start RET
> and  from the same terminal do:
> emacsclient -t
>
> This results in:
> [1]  + Suspended (tty output)        emacs -Q
>
> This is a regression, it works just fine in 24.1

The commit that caused this is 110801.

This hunk looks suspicious:

@@ -1286,11 +1282,7 @@ child_setup (int in, int out, int err, c
   if (err != in && err != out)
     emacs_close (err);

-#if defined HAVE_SETPGID || ! (defined USG && defined SETPGRP_RELEASES_CTTY)
-  setpgid (pid, pid);
-#endif
-
-  /* setpgrp_of_tty is incorrect here; it uses input_fd.  */
+  setpgid (0, 0);
   tcsetpgrp (0, pid);
 
   /* execvp does not accept an environment arg so the only way


it replaces setpgid (pid, pid) with setpgid (0, 0) on GNU/Linux.

bug#13387: emacsclient -t suspends emacs

[Andreas Schwab]

Dan Nicolaescu <dann@gnu.org> writes:

> it replaces setpgid (pid, pid) with setpgid (0, 0) on GNU/Linux.

Both do exactly the same.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#13387: emacsclient -t suspends emacs

[Dan Nicolaescu]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Dan Nicolaescu <dann@gnu.org> writes:
>
>> it replaces setpgid (pid, pid) with setpgid (0, 0) on GNU/Linux.
>
> Both do exactly the same.

Thanks for eliminating that possibility.

I can confirm that the problem is caused by this change:

@@ -2918,36 +2914,9 @@ set_tty_hooks (struct terminal *terminal
 static void
 dissociate_if_controlling_tty (int fd)
 {
-#ifndef DOS_NT
   pid_t pgid = tcgetpgrp (fd); /* If tcgetpgrp succeeds, fd is the ctty. */
-  if (pgid != -1)
-    {
-#if defined (USG5)
-      setpgrp ();
-      no_controlling_tty = 1;
-#elif defined (CYGWIN)
-      setsid ();
-      no_controlling_tty = 1;
-#else
-#ifdef TIOCNOTTY                /* Try BSD ioctls. */
-      sigset_t blocked;
-      sigemptyset (&blocked);
-      sigaddset (&blocked, SIGTTOU);
-      pthread_sigmask (SIG_BLOCK, &blocked, 0);
-      fd = emacs_open (DEV_TTY, O_RDWR, 0);
-      if (fd != -1 && ioctl (fd, TIOCNOTTY, 0) != -1)
-        {
-          no_controlling_tty = 1;
-        }
-      if (fd != -1)
-        emacs_close (fd);
-      pthread_sigmask (SIG_UNBLOCK, &blocked, 0);
-#else
-# error "Unknown system."
-#endif  /* ! TIOCNOTTY */
-#endif  /* ! USG */
-    }
-#endif /* !DOS_NT */
+  if (0 <= pgid)
+    setsid ();
 }
 
 /* Create a termcap display on the tty device with the given name and


Also I think that the "USG5" case did not work in the original code, I
vaguely remember that this problem happened on Solaris a few years ago.
I don't have easy access to a Solaris machine to confirm....

bug#13387: emacsclient -t suspends emacs

[Paul Eggert]

On 01/10/2013 05:43 AM, Dan Nicolaescu wrote:
> I can confirm that the problem is caused by this change: ...
> Also I think that the "USG5" case did not work in the original code

Thanks for the diagnosis.  I installed what I hope is a fix for the
problem as trunk bzr 111478; please give it a try.  I don't know how
to fix the problem on platforms that lack TIOCNOTTY, but perhaps platforms
that lack TIOCNOTTY won't have the problem.  I can see why the old USG5
code didn't work; the code I installed shouldn't have that problem
(for platforms that have TIOCNOTTY).

bug#13387: emacsclient -t suspends emacs

[Dan Nicolaescu]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Dan Nicolaescu <dann@gnu.org> writes:
>
>> it replaces setpgid (pid, pid) with setpgid (0, 0) on GNU/Linux.
>
> Both do exactly the same.

BTW, I think you maintain the git mirror.  I first tried to used git
bisect to find the origin of this issue, and could now.
Now that the bad commit has been identified I tried to use git bisect again to
see if it can find it.

I set the good version to be the one before the problem commit, and then
run 
git bisect bad
git bisect bad
git bisect bad
[etc]

It should have stopped and the bad commit, instead it stopped at a different commit.

Am I doing something wrong? Is there's a problem in the repository?
Here's what
git bisect log
says:

git bisect start
# bad: [61e4ca21b70beb2a0402d2afb0c33855097f7612] * lisp/files.el (auto-mode-alist): Use doc-view for djvu files. * lisp/doc-view.el (doc-view-document->bitmap): Use doc-view-single-page-converter-function instead of single-page-converter arg; adjust callers.
git bisect bad 61e4ca21b70beb2a0402d2afb0c33855097f7612
# good: [56985d81b48ffa443e4603de84d8856325aa56f1] Remove EMACS_OUTQSIZE+sleep hack.
git bisect good 56985d81b48ffa443e4603de84d8856325aa56f1
# bad: [9e1ba9c2b816823aa8982fb5ba1fa7bc8171bb71] Merge from emacs-24; up to r110999
git bisect bad 9e1ba9c2b816823aa8982fb5ba1fa7bc8171bb71
# skip: [8c4a99947a3f0ca54e80c57497c203d99089767c] Add a menu for profiler report mode
git bisect skip 8c4a99947a3f0ca54e80c57497c203d99089767c
# bad: [8c4a99947a3f0ca54e80c57497c203d99089767c] Add a menu for profiler report mode
git bisect bad 8c4a99947a3f0ca54e80c57497c203d99089767c
# bad: [751637b60f8f64f95c56f671ccbcdd4ee26fb99e] Merge from emacs-24; up to r110855
git bisect bad 751637b60f8f64f95c56f671ccbcdd4ee26fb99e
# bad: [605f7ff983481998e7d08a1d5959da9ea549164e] * lisp/emacs-lisp/gv.el (gv-define-simple-setter): One more fix.
git bisect bad 605f7ff983481998e7d08a1d5959da9ea549164e
# bad: [b0dd821ad4952755f440d998643c9966a00404a5] Rewrite and rename diff-delete-trailing-whitespace.
git bisect bad b0dd821ad4952755f440d998643c9966a00404a5
# bad: [c703fb479c0007a198f0b6cdea6e66f2c84b2b31] * lisp/emacs-lisp/gv.el (gv-define-simple-setter): Don't evaluate `val' twice when `fix-return' is set.
git bisect bad c703fb479c0007a198f0b6cdea6e66f2c84b2b31
# bad: [2e14f71abbac30fe01117cd352393ee8d361f9f6] * lisp/woman.el (woman-decode-region): Disable adaptive-fill when rendering.
git bisect bad 2e14f71abbac30fe01117cd352393ee8d361f9f6
# bad: [20060d7c16723e43aa3a1c260e9d3b9054c069a0] * frames.texi (Initial Parameters): Doc fix.
git bisect bad 20060d7c16723e43aa3a1c260e9d3b9054c069a0
# bad: [1ce467b17adbab9db6d87dfc427b42330cc3d38b] Auto-commit of loaddefs files.
git bisect bad 1ce467b17adbab9db6d87dfc427b42330cc3d38b
# bad: [51747530361f3eb823040f5e011cb6569153f7f6] Auto-commit of loaddefs files.
git bisect bad 51747530361f3eb823040f5e011cb6569153f7f6
# bad: [51747530361f3eb823040f5e011cb6569153f7f6] Auto-commit of loaddefs files.
git bisect bad 51747530361f3eb823040f5e011cb6569153f7f6

bug#13387: emacsclient -t suspends emacs

[Dan Nicolaescu]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 01/10/2013 05:43 AM, Dan Nicolaescu wrote:
>> I can confirm that the problem is caused by this change: ...
>> Also I think that the "USG5" case did not work in the original code
>
> Thanks for the diagnosis.  I installed what I hope is a fix for the
> problem as trunk bzr 111478; please give it a try.  I don't know how
> to fix the problem on platforms that lack TIOCNOTTY, but perhaps platforms
> that lack TIOCNOTTY won't have the problem.  I can see why the old USG5
> code didn't work; the code I installed shouldn't have that problem
> (for platforms that have TIOCNOTTY).

It works fine on GNU/Linux, I did not try it anywhere else.
Thanks!

bug#13387: emacsclient -t suspends emacs

[Andreas Schwab]

Dan Nicolaescu <dann@gnu.org> writes:

> Am I doing something wrong?

I can easily happen that two bugs overlap each other, causing a wrong
resolution by mistake.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#13387: emacsclient -t suspends emacs

[Dan Nicolaescu]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Dan Nicolaescu <dann@gnu.org> writes:
>
>> Am I doing something wrong?
>
> I can easily happen that two bugs overlap each other, causing a wrong
> resolution by mistake.

Sorry, can you please explain what you mean?
The log I shown involved no testing, just running git commands to see if
it converges to the right point (which is now known...)...

bug#13387: emacsclient -t suspends emacs

[Andreas Schwab]

Dan Nicolaescu <dann@gnu.org> writes:

> The log I shown involved no testing,

Then how do you know whether the commit is good or bad?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#13387: emacsclient -t suspends emacs

[Dan Nicolaescu]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Dan Nicolaescu <dann@gnu.org> writes:
>
>> The log I shown involved no testing,
>
> Then how do you know whether the commit is good or bad?

Because I've identified the bad commit otherwise.
Then I set 
git bisect good BLAH

where BLAH is the commit just before the bad one.

doing a sequence of
git bisect bad

should converge on the bad commit, not before that, which is what
happened in this case.

bug#13387: emacsclient -t suspends emacs

[Andreas Schwab]

If you misidentify a commit you cannot expect to get any useful result.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#13387: emacsclient -t suspends emacs

[Dan Nicolaescu]

Andreas Schwab <schwab@linux-m68k.org> writes:

> If you misidentify a commit you cannot expect to get any useful result.

I don't think I misidentified anything.

Can you please try this exercise?
The bad commit is 1f8a924624b967c7626ce3f06e6ee3014ad42d2b
Anything after it is bad.
The one before it, f0d19abcc35c104cdf956caecce314bf456a5e3a (and anything before it) is good.

With these, I don't get git bisect to converge on the bad commit...

bug#13387: emacsclient -t suspends emacs

[Andreas Schwab]

Dan Nicolaescu <dann@gnu.org> writes:

> With these, I don't get git bisect to converge on the bad commit...

That can only happen if you have misattributed a commit.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#13387: emacsclient -t suspends emacs

[Dan Nicolaescu]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Dan Nicolaescu <dann@gnu.org> writes:
>
>> With these, I don't get git bisect to converge on the bad commit...
>
> That can only happen if you have misattributed a commit.

I don't think I have.  See the what happens below.

The bad commit is: 1f8a924.


$ git bisect start
$ git bisect bad
$ git bisect good f0d19ab
Bisecting: 598 revisions left to test after this (roughly 9 steps)
[28e19080148146cf7cdb42cf7139e8ccfb29560a] * info.el (Info-set-mode-line): Remove file extension from Info-current-file if there is one.
$ git bisect bad
Bisecting: 300 revisions left to test after this (roughly 8 steps)
[b382a63696f3941f207310576c1fe2edd3dc875b] Conflate Qnil and Qunbound for `symbol-function'. * src/alloc.c (Fmake_symbol): Initialize `function' to Qnil. * src/lread.c (init_obarray): Set `function' fields to Qnil. * src/eval.c (Fcommandp): Ignore Qunbound. (Fautoload, eval_sub, Fapply, Ffuncall, Fmacroexpand): * src/data.c (Ffset, Ffboundp, indirect_function, Findirect_function): Test NILP rather than Qunbound. (Ffmakunbound): Set to Qnil. (Fsymbol_function): Never signal an error. (Finteractive_form): Ignore Qunbound.
$ git bisect bad
Bisecting: 143 revisions left to test after this (roughly 7 steps)
[751637b60f8f64f95c56f671ccbcdd4ee26fb99e] Merge from emacs-24; up to r110855
$ git bisect bad
Bisecting: 76 revisions left to test after this (roughly 6 steps)
[2ac3bd2e7270f263f0eb9cae8515cb1206db2b27] Doc fix for gdb-stopped-functions.
$ git bisect bad
Bisecting: 37 revisions left to test after this (roughly 5 steps)
[b0dd821ad4952755f440d998643c9966a00404a5] Rewrite and rename diff-delete-trailing-whitespace.
$ git bisect bad
Bisecting: 18 revisions left to test after this (roughly 4 steps)
[c703fb479c0007a198f0b6cdea6e66f2c84b2b31] * lisp/emacs-lisp/gv.el (gv-define-simple-setter): Don't evaluate `val' twice when `fix-return' is set.
$ git bisect bad
Bisecting: 9 revisions left to test after this (roughly 3 steps)
[a42511a0cc14f22cec63cdab500e5eebbc63c185] Add some documentation on defining new generalized variables
$ git bisect bad
Bisecting: 4 revisions left to test after this (roughly 2 steps)
[b8f1b1dfdd9b7a173d92b2bf47026fbbcee33a41] * dired.texi (Dired Updating): Doc fix.
$ git bisect bad
Bisecting: 1 revision left to test after this (roughly 1 step)
[ce494c0100321e5e571999c50f808679f4798f47] Misc small cl doc fixes
$ git bisect bad
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[51747530361f3eb823040f5e011cb6569153f7f6] Auto-commit of loaddefs files.

^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is before the good commit.  Why?  Let's say it's good anyway:

$ git bisect good
ce494c0100321e5e571999c50f808679f4798f47 is the first bad commit
commit ce494c0100321e5e571999c50f808679f4798f47
Author: Glenn Morris <rgm@gnu.org>
Date:   Mon Nov 5 00:29:12 2012 -0800

    Misc small cl doc fixes
    
    * emacs-lisp/cl-extra.el (cl-maplist, cl-mapcan): Doc fix.
    
    * emacs-lisp/cl-extra.el (cl-prettyexpand):
    * emacs-lisp/cl-lib.el (cl-proclaim, cl-declaim):
    * emacs-lisp/cl-macs.el (cl-destructuring-bind, cl-locally)
    (cl-the, cl-compiler-macroexpand): Add basic doc strings.

:040000 040000 e8dd59be7c6b38dbc1bd0372ae8d9294ada0b354 90653dde40314ec57b022dc389f234c245d1091c M      lisp

bug#13387: emacsclient -t suspends emacs

[Andreas Schwab]

Dan Nicolaescu <dann@gnu.org> writes:

> [2ac3bd2e7270f263f0eb9cae8515cb1206db2b27] Doc fix for gdb-stopped-functions.
> $ git bisect bad

That cannot be right.

$ git merge-base 1f8a924 2ac3bd2e7270f263f0eb9cae8515cb1206db2b27
a6f9c40d23ebbcd72a273cef06f9414743222c95

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#13387: emacsclient -t suspends emacs

[Dan Nicolaescu]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Dan Nicolaescu <dann@gnu.org> writes:
>
>> [2ac3bd2e7270f263f0eb9cae8515cb1206db2b27] Doc fix for gdb-stopped-functions.
>> $ git bisect bad
>
> That cannot be right.
>
> $ git merge-base 1f8a924 2ac3bd2e7270f263f0eb9cae8515cb1206db2b27
> a6f9c40d23ebbcd72a273cef06f9414743222c95

Thanks, interesting...