Re: [angeloff@acm.org: Emacs bug with Solaris 9 (only)]

Re: [angeloff@acm.org: Emacs bug with Solaris 9 (only)]

[Jan D.]

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

> In case anyone here can fix it...
> 
> ------- Start of forwarded message -------
> X-Authentication-Warning: host132.grad.iit.edu: angeloff owned process doing
> 	-bs
> Date: Wed, 22 Jan 2003 00:23:14 -0600 (CST)
> From: Nikolay Angeloff <angeloff@acm.org>
> X-X-Sender: <angeloff@host132.grad.iit.edu>
> To: <bug-gnu-emacs@prep.ai.mit.edu>
> cc: rms-assist@gnu.org
> Subject: Emacs bug with Solaris 9 (only)
> Sender: bug-gnu-emacs-bounces+rms=gnu.org@gnu.org
> 
> In short: quit-process, stop-process, get-process etc.
> do not work when emacs is executed (and compiled) in
> Solaris 9, but work just fine when the same compiled
> code is scp-ed to a Solaris 8 machine.
> 

It seems that ioctl TICSIGNAL is broken in Solaris 9.  Emacs uses that
to send signals to the slave side of the pty.  I propose that Emacs
check the return code of ioctl and falls back to killing the process
group if the ioctl fails (see patch below).

Another thing is that getting the tty foreground process group does
not work when using the master side file descriptor, the slave side
descriptor must be used.  This is true for Solaris 8 also, but there
TICSIGNAL works so it is harder to notice.'

In process there are two places that gets the terminal process group,
process_send_signal and Fprocess_running_child_p.  Both places look 
like this:

  if (!NILP (p->subtty))
    ioctl (XFASTINT (p->subtty), TIOCGPGRP, &gid);
  else
    ioctl (XINT (p->infd), TIOCGPGRP, &gid);

Now, reading the code it seems to me that p->subtty is unconditionally
set to Qnil when the fork of the subprocess succeeds, so the first
ioctl will never be executed.  Am I missing something?

I propose reopening the slave side in Emacs to get the process group.
This works in Solaris 8 and 9 and should have no sideeffects
on other systems.

The attached patch fixes just one place, a better patch would be to
introduce an emacs_get_tty_pgrp or something and call that from
these two places.  The attached patch fixes the fact that you can not
do
  M-x shell
  sleep 12
  C-c C-c

and have the sleep interrupted.  If this approach is acceptable, I can
make a better patch.  Also, this should go into RC (the patch is for RC).

But I would like to know if the "if (!NILP (p->subtty))" code ever will
be TRUE before committing anything.

	Jan D.


[-- Attachment #2: process.c.diff --]
[-- Type: text/plain, Size: 1172 bytes --]

*** process.c	ons feb 12 22:24:25 2003
--- emacsRC/emacs/src/process.c	lör feb 15 13:22:21 2003
***************
*** 3755,3760 ****
--- 3755,3772 ----
  	else
  	  err = ioctl (XINT (p->infd), TIOCGPGRP, &gid);
  
+ 	if (err == -1 && NILP (p->subtty) && ! NILP (p->tty_name))
+ 	  {
+ 	    int fd;
+ 	    fd = emacs_open (XSTRING (p->tty_name)->data, O_RDONLY, 0);
+ 
+ 	    if (fd != -1)
+ 	      {
+ 		err = ioctl (fd, TIOCGPGRP, &gid);
+ 		emacs_close (fd);
+ 	      }
+ 	  }
+ 
  #ifdef pfa
  	if (err == -1)
  	  gid = - XFASTINT (p->pid);
***************
*** 3821,3827 ****
    /* gid may be a pid, or minus a pgrp's number */
  #ifdef TIOCSIGSEND
    if (!NILP (current_group))
!     ioctl (XINT (p->infd), TIOCSIGSEND, signo);
    else
      {
        gid = - XFASTINT (p->pid);
--- 3833,3845 ----
    /* gid may be a pid, or minus a pgrp's number */
  #ifdef TIOCSIGSEND
    if (! NILP (current_group))
!     {
!       if (ioctl (XINT (p->infd), TIOCSIGSEND, signo) == -1)
! 	{
! 	  EMACS_KILLPG (gid, signo);
! 	  fprintf(stderr, "Kill 2: %d\n", gid);
! 	}
!     }
    else
      {
        gid = - XFASTINT (p->pid);

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: [angeloff@acm.org: Emacs bug with Solaris 9 (only)]

[Richard Stallman]

    Now, reading the code it seems to me that p->subtty is unconditionally
    set to Qnil when the fork of the subprocess succeeds, so the first
    ioctl will never be executed.

Here's the code that sets subtty.

  /* Record the tty descriptor used in the subprocess.  */
  if (forkin < 0)
    XPROCESS (process)->subtty = Qnil;
  else
    XSETFASTINT (XPROCESS (process)->subtty, forkin);

It doesn't look like this always sets it to nil, unless
forkin is always < 0.  Basically, it is not SUPPOSED
to be always nil, so if you find that it always IS nil,
there must be a bug somewhere.

    The attached patch fixes just one place, a better patch would be to
    introduce an emacs_get_tty_pgrp or something and call that from
    these two places.

Would you like to do that?

Re: [angeloff@acm.org: Emacs bug with Solaris 9 (only)]

[Jan D.]

>     Now, reading the code it seems to me that p->subtty is unconditionally
>     set to Qnil when the fork of the subprocess succeeds, so the first
>     ioctl will never be executed.
>
> Here's the code that sets subtty.
>
>   /* Record the tty descriptor used in the subprocess.  */
>   if (forkin < 0)
>     XPROCESS (process)->subtty = Qnil;
>   else
>     XSETFASTINT (XPROCESS (process)->subtty, forkin);
>
> It doesn't look like this always sets it to nil, unless
> forkin is always < 0.  Basically, it is not SUPPOSED
> to be always nil, so if you find that it always IS nil,
> there must be a bug somewhere.

But later at the end of the same function (create_process) we have:

       /* vfork succeeded.  */
       ...

         XPROCESS (process)->subtty = Qnil;
         if (forkin >= 0)
           emacs_close (forkin);


	Jan D.

Re: [angeloff@acm.org: Emacs bug with Solaris 9 (only)]

[Richard Stallman]

    But later at the end of the same function (create_process) we have:

	   /* vfork succeeded.  */
	   ...

	     XPROCESS (process)->subtty = Qnil;
	     if (forkin >= 0)
	       emacs_close (forkin);

This code is really self-contradictory.

I studied the code some more.  The idea of subtty seems to be that it
holds the terminal descriptor that the child should use.  However, the
code you cite above seems to assume that descriptor needs to be
closed.  That code seems to have been present since 1994.
However, in version 1.1 of process.c it looked like this:

    #ifdef SYSV4_PTYS
      /* OK to close only if it's not a pty.  Otherwise we need to leave
	 it open for ioctl to get pgrp when signals are sent, or to send
	 the interrupt characters through if that's how we're signalling
	 subprocesses.  Alternately if you are concerned about running out
	 of file descriptors, you could just save the tty name and open
	 just to do the ioctl.  */
      if (NILP (XFASTINT (XPROCESS (process)->pty_flag)))
    #endif
	{
	  XPROCESS (process)->subtty = Qnil;
	  if (forkin >= 0)
	    close (forkin);
	}

It looks like subtty is obsolete.  We could delete it,
unless there is some system on which really using it would
solve some problem today.

Re: [angeloff@acm.org: Emacs bug with Solaris 9 (only)]

[Markus Rost]

I just tried your patch (for CVS) on Solaris 9, without trying to
understand it.  So far it works well.  Thanks.


Index: src/process.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/process.c,v
retrieving revision 1.399
diff -c -r1.399 process.c
*** src/process.c	10 Feb 2003 13:51:43 -0000	1.399
--- src/process.c	15 Feb 2003 16:57:10 -0000
***************

Re: [angeloff@acm.org: Emacs bug with Solaris 9 (only)]

[Jan D.]

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

Hello.

Attached are the patches I think should go in, provided my observation
that p->subtty is always Qnil is correct.

	Jan D.


[-- Attachment #2: emacsRC.diff --]
[-- Type: text/plain, Size: 3676 bytes --]

Index: src/process.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/process.c,v
retrieving revision 1.341.4.3
diff -c -r1.341.4.3 process.c
*** src/process.c	23 Oct 2002 17:40:15 -0000	1.341.4.3
--- src/process.c	15 Feb 2003 16:56:35 -0000
***************
*** 3571,3576 ****
--- 3571,3603 ----
    return Qnil;
  }
  \f
+ /* Return the foreground process group for the tty/pty that
+    the process P uses.  */
+ static int
+ emacs_get_tty_pgrp (p)
+      struct Lisp_Process *p;
+ {
+   int gid = -1;
+ 
+ #ifdef TIOCGPGRP 
+   if (ioctl (XINT (p->infd), TIOCGPGRP, &gid) == -1 && ! NILP (p->tty_name))
+     {
+       int fd;
+       /* Some OS:es (Solaris 8/9) does not allow TIOCGPGRP from the
+ 	 master side.  Try the slave side.  */
+       fd = emacs_open (XSTRING (p->tty_name)->data, O_RDONLY, 0);
+ 
+       if (fd != -1)
+ 	{
+ 	  ioctl (fd, TIOCGPGRP, &gid);
+ 	  emacs_close (fd);
+ 	}
+     }
+ #endif /* defined (TIOCGPGRP ) */
+ 
+   return gid;
+ }
+ 
  DEFUN ("process-running-child-p", Fprocess_running_child_p,
         Sprocess_running_child_p, 0, 1, 0,
    "Return t if PROCESS has given the terminal to a child.\n\
***************
*** 3581,3587 ****
  {
    /* Initialize in case ioctl doesn't exist or gives an error,
       in a way that will cause returning t.  */
!   int gid = 0;
    Lisp_Object proc;
    struct Lisp_Process *p;
  
--- 3608,3614 ----
  {
    /* Initialize in case ioctl doesn't exist or gives an error,
       in a way that will cause returning t.  */
!   int gid;
    Lisp_Object proc;
    struct Lisp_Process *p;
  
***************
*** 3595,3606 ****
      error ("Process %s is not active",
  	   XSTRING (p->name)->data);
  
! #ifdef TIOCGPGRP 
!   if (!NILP (p->subtty))
!     ioctl (XFASTINT (p->subtty), TIOCGPGRP, &gid);
!   else
!     ioctl (XINT (p->infd), TIOCGPGRP, &gid);
! #endif /* defined (TIOCGPGRP ) */
  
    if (gid == XFASTINT (p->pid))
      return Qnil;
--- 3622,3628 ----
      error ("Process %s is not active",
  	   XSTRING (p->name)->data);
  
!   gid = emacs_get_tty_pgrp (p);
  
    if (gid == XFASTINT (p->pid))
      return Qnil;
***************
*** 3747,3765 ****
  	 But, TIOCGPGRP does not work on E50 ;-P works fine on E60"
  	 His patch indicates that if TIOCGPGRP returns an error, then
  	 we should just assume that p->pid is also the process group id.  */
-       {
- 	int err;
  
! 	if (!NILP (p->subtty))
! 	  err = ioctl (XFASTINT (p->subtty), TIOCGPGRP, &gid);
! 	else
! 	  err = ioctl (XINT (p->infd), TIOCGPGRP, &gid);
  
  #ifdef pfa
! 	if (err == -1)
! 	  gid = - XFASTINT (p->pid);
  #endif /* ! defined (pfa) */
!       }
        if (gid == -1)
  	no_pgrp = 1;
        else
--- 3769,3782 ----
  	 But, TIOCGPGRP does not work on E50 ;-P works fine on E60"
  	 His patch indicates that if TIOCGPGRP returns an error, then
  	 we should just assume that p->pid is also the process group id.  */
  
!       gid = emacs_get_tty_pgrp (p);
  
  #ifdef pfa
!       if (gid == -1)
!         gid = - XFASTINT (p->pid);
  #endif /* ! defined (pfa) */
! 
        if (gid == -1)
  	no_pgrp = 1;
        else
***************
*** 3821,3827 ****
    /* gid may be a pid, or minus a pgrp's number */
  #ifdef TIOCSIGSEND
    if (!NILP (current_group))
!     ioctl (XINT (p->infd), TIOCSIGSEND, signo);
    else
      {
        gid = - XFASTINT (p->pid);
--- 3838,3847 ----
    /* gid may be a pid, or minus a pgrp's number */
  #ifdef TIOCSIGSEND
    if (!NILP (current_group))
!     {
!       if (ioctl (XINT (p->infd), TIOCSIGSEND, signo) == -1)
! 	EMACS_KILLPG (-gid, signo);
!     }
    else
      {
        gid = - XFASTINT (p->pid);

[-- Attachment #3: emacsCVS.diff --]
[-- Type: text/plain, Size: 3894 bytes --]

Index: src/process.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/process.c,v
retrieving revision 1.399
diff -c -r1.399 process.c
*** src/process.c	10 Feb 2003 13:51:43 -0000	1.399
--- src/process.c	15 Feb 2003 16:57:10 -0000
***************
*** 5104,5109 ****
--- 5104,5136 ----
    return Qnil;
  }
  \f
+ /* Return the foreground process group for the tty/pty that
+    the process P uses.  */
+ static int
+ emacs_get_tty_pgrp (p)
+      struct Lisp_Process *p;
+ {
+   int gid = -1;
+ 
+ #ifdef TIOCGPGRP 
+   if (ioctl (XINT (p->infd), TIOCGPGRP, &gid) == -1 && ! NILP (p->tty_name))
+     {
+       int fd;
+       /* Some OS:es (Solaris 8/9) does not allow TIOCGPGRP from the
+ 	 master side.  Try the slave side.  */
+       fd = emacs_open (XSTRING (p->tty_name)->data, O_RDONLY, 0);
+ 
+       if (fd != -1)
+ 	{
+ 	  ioctl (fd, TIOCGPGRP, &gid);
+ 	  emacs_close (fd);
+ 	}
+     }
+ #endif /* defined (TIOCGPGRP ) */
+ 
+   return gid;
+ }
+ 
  DEFUN ("process-running-child-p", Fprocess_running_child_p,
         Sprocess_running_child_p, 0, 1, 0,
         doc: /* Return t if PROCESS has given the terminal to a child.
***************
*** 5114,5120 ****
  {
    /* Initialize in case ioctl doesn't exist or gives an error,
       in a way that will cause returning t.  */
!   int gid = 0;
    Lisp_Object proc;
    struct Lisp_Process *p;
  
--- 5141,5147 ----
  {
    /* Initialize in case ioctl doesn't exist or gives an error,
       in a way that will cause returning t.  */
!   int gid;
    Lisp_Object proc;
    struct Lisp_Process *p;
  
***************
*** 5128,5139 ****
      error ("Process %s is not active",
  	   SDATA (p->name));
  
! #ifdef TIOCGPGRP
!   if (!NILP (p->subtty))
!     ioctl (XFASTINT (p->subtty), TIOCGPGRP, &gid);
!   else
!     ioctl (XINT (p->infd), TIOCGPGRP, &gid);
! #endif /* defined (TIOCGPGRP ) */
  
    if (gid == XFASTINT (p->pid))
      return Qnil;
--- 5155,5161 ----
      error ("Process %s is not active",
  	   SDATA (p->name));
  
!   gid = emacs_get_tty_pgrp (p);
  
    if (gid == XFASTINT (p->pid))
      return Qnil;
***************
*** 5285,5303 ****
  	 But, TIOCGPGRP does not work on E50 ;-P works fine on E60"
  	 His patch indicates that if TIOCGPGRP returns an error, then
  	 we should just assume that p->pid is also the process group id.  */
-       {
- 	int err;
  
! 	if (!NILP (p->subtty))
! 	  err = ioctl (XFASTINT (p->subtty), TIOCGPGRP, &gid);
! 	else
! 	  err = ioctl (XINT (p->infd), TIOCGPGRP, &gid);
! 
! 	if (err == -1)
! 	  /* If we can't get the information, assume
! 	     the shell owns the tty.  */
! 	  gid = XFASTINT (p->pid);
!       }
  
        /* It is not clear whether anything really can set GID to -1.
  	 Perhaps on some system one of those ioctls can or could do so.
--- 5307,5319 ----
  	 But, TIOCGPGRP does not work on E50 ;-P works fine on E60"
  	 His patch indicates that if TIOCGPGRP returns an error, then
  	 we should just assume that p->pid is also the process group id.  */
  
!       gid = emacs_get_tty_pgrp (p);
! 	
!       if (gid == -1)
! 	/* If we can't get the information, assume
! 	   the shell owns the tty.  */
! 	gid = XFASTINT (p->pid);
  
        /* It is not clear whether anything really can set GID to -1.
  	 Perhaps on some system one of those ioctls can or could do so.
***************
*** 5359,5365 ****
    /* gid may be a pid, or minus a pgrp's number */
  #ifdef TIOCSIGSEND
    if (!NILP (current_group))
!     ioctl (XINT (p->infd), TIOCSIGSEND, signo);
    else
      {
        gid = - XFASTINT (p->pid);
--- 5375,5384 ----
    /* gid may be a pid, or minus a pgrp's number */
  #ifdef TIOCSIGSEND
    if (!NILP (current_group))
!     {
!       if (ioctl (XINT (p->infd), TIOCSIGSEND, signo) == -1)
! 	EMACS_KILLPG (gid, signo);
!     }
    else
      {
        gid = - XFASTINT (p->pid);

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

[angeloff@acm.org: Emacs bug with Solaris 9 (only)]

[Markus Rost]

I have no time to debug this.

However, let me at least confirm that I experience since a while
exactly the same problem on Solaris 9.  I don't remember whether the
problem was not present on Solaris 8, and I cannot test this anymore,
but this is entirely possible.

Actually it is annoying:  You come to kill the buffer *shell* in order
to kill a subprocess...

Precise Example:  Call M-x shell.  Then in the shell start a
subprocess like xterm or emacs.  Then C-c C-c (which runs the command
comint-interrupt-subjob) does not kill the subprocess.  Nor does
<menu-bar> <signals> <kill> (which runs the command
comint-kill-subjob).


   In case anyone here can fix it...

   ------- Start of forwarded message -------
   X-Authentication-Warning: host132.grad.iit.edu: angeloff owned process doing
	   -bs
   Date: Wed, 22 Jan 2003 00:23:14 -0600 (CST)
   From: Nikolay Angeloff <angeloff@bogus.example.com>
   X-X-Sender: <angeloff@bogus.example.com>
   To: <bug-gnu-emacs@bogus.example.com>
   cc: rms-assist@bogus.example.com
   Subject: Emacs bug with Solaris 9 (only)
   Sender: bug-gnu-emacs-bounces+rms=gnu.org@bogus.example.com

   In short: quit-process, stop-process, get-process etc.
   do not work when emacs is executed (and compiled) in
   Solaris 9, but work just fine when the same compiled
   code is scp-ed to a Solaris 8 machine.

   Because of some <unknown to me> differences between
   Solaris 8 and Solaris 9 the error I described occurs.
   Any ideas? I saw a post of a similar message, but did
   not see any answers to it.

   I noticed this problem, when in tex mode, I could not kill
   my buffer.

   Nikolay Angeloff

[angeloff@acm.org: Emacs bug with Solaris 9 (only)]

[Richard Stallman]

In case anyone here can fix it...

------- Start of forwarded message -------
X-Authentication-Warning: host132.grad.iit.edu: angeloff owned process doing
	-bs
Date: Wed, 22 Jan 2003 00:23:14 -0600 (CST)
From: Nikolay Angeloff <angeloff@acm.org>
X-X-Sender: <angeloff@host132.grad.iit.edu>
To: <bug-gnu-emacs@prep.ai.mit.edu>
cc: rms-assist@gnu.org
Subject: Emacs bug with Solaris 9 (only)
Sender: bug-gnu-emacs-bounces+rms=gnu.org@gnu.org

In short: quit-process, stop-process, get-process etc.
do not work when emacs is executed (and compiled) in
Solaris 9, but work just fine when the same compiled
code is scp-ed to a Solaris 8 machine.

Because of some <unknown to me> differences between
Solaris 8 and Solaris 9 the error I described occurs.
Any ideas? I saw a post of a similar message, but did
not see any answers to it.

I noticed this problem, when in tex mode, I could not kill
my buffer.

Nikolay Angeloff



_______________________________________________
Bug-gnu-emacs mailing list
Bug-gnu-emacs@gnu.org
http://mail.gnu.org/mailman/listinfo/bug-gnu-emacs
------- End of forwarded message -------