BLOCK_INPUT on Mac OS X

BLOCK_INPUT on Mac OS X

[YAMAMOTO Mitsuharu]

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

I'm now trying to fill missing BLOCK_INPUTs for Mac OS X.  I suspect
occasional hang of Carbon Emacs is due to the heap corruption caused
by interrupted malloc/free mentioned in blockinput.h.

Two patches are attached.  The first one is for adding missing
BLOCK_INPUTs that I tracked down (mainly for window operations).  The
second one is for a helper that assists us to detect
malloc/calloc/valloc/realloc/free that is not protected by
BLOCK_INPUT.  It hooks a check function to each internal memory
allocation routine.  To use this, compile with "CFLAGS=-g
-DDEBUG_BLOCK_INPUT" after applying both patches, and set a break
point to `debug_block_input'.  It works only on Carbon build.

I have some questions about this issue.

1. The check function that is executed in every malloc call is as
   follows:

    int
    check_block_input ()
    {
      sigset_t mask;

      if (poll_suppress_count == 0 && interrupt_input_blocked == 0)
	{
	  sigprocmask (0, NULL, &mask);
	  if (!sigismember (&mask, SIGALRM))
	   {
	     debug_block_input ();
	     return -1;
	   }
	}
      return 0;
    }

   Is this correct?  Or does it produce a false alarm?
   (Carbon Emacs uses polling by SIGALRM, because window events don't
    come from sockets and we can't use SIGIO.)

2. The following library functions are detected as those that may call
   malloc and so on without BLOCK_INPUT.

   localtime, gmtime, ctime, opendir, getc, getaddrinfo, fwrite, mkstemp
   fclose, closedir, freeaddrinfo

   If the check function in the previous item is correct, should we
   make a wrapper function for each of them?  Or add BLOCK_INPUTs to
   caller functions?

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

[-- Attachment #2: diff-blockinput.gz --]
[-- Type: application/octet-stream, Size: 2367 bytes --]

[-- Attachment #3: diff-blockinput-debug.gz --]
[-- Type: application/octet-stream, Size: 1782 bytes --]

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

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

Re: BLOCK_INPUT on Mac OS X

[Kim F. Storm]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

> 2. The following library functions are detected as those that may call
>    malloc and so on without BLOCK_INPUT.
> 
>    localtime, gmtime, ctime, opendir, getc, getaddrinfo, fwrite, mkstemp
>    fclose, closedir, freeaddrinfo
> 
>    If the check function in the previous item is correct, should we
>    make a wrapper function for each of them?  Or add BLOCK_INPUTs to
>    caller functions?

Interesting...

If MAC Carbon has problems due to missing BLOCK_INPUT for these
functions, I would imagine that similar problems would be seen on
other systems as well.

Maybe this can explain some of the strange crashes that are (were?)
reported from time to time on some systems?

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: BLOCK_INPUT on Mac OS X

[Stefan Monnier]

> I'm now trying to fill missing BLOCK_INPUTs for Mac OS X.  I suspect
> occasional hang of Carbon Emacs is due to the heap corruption caused
> by interrupted malloc/free mentioned in blockinput.h.

If you don't use SIGIO, I can't see how malloc/free could be interrupted.
What other signals are used?

Also, do a grep for "SYNC_INPUT", which is a compilation option I use to
make the Xwindow code move most of the processing to outside of the signal
handler (when compiled with SYNC_INPUT, the "interrupted malloc" should
simply never be a problem and the BLOCK_INPUTS there are unnecessary).


        Stefan

Re: BLOCK_INPUT on Mac OS X

[YAMAMOTO Mitsuharu]

>>>>> On 03 Sep 2004 12:48:19 -0400, Stefan Monnier <monnier@iro.umontreal.ca> said:

> If you don't use SIGIO, I can't see how malloc/free could be
> interrupted.  What other signals are used?

SIGALRM, for polling input events periodically.  This is basically the
same way with what Emacs on Solaris is doing.

> Also, do a grep for "SYNC_INPUT", which is a compilation option I
> use to make the Xwindow code move most of the processing to outside
> of the signal handler (when compiled with SYNC_INPUT, the
> "interrupted malloc" should simply never be a problem and the
> BLOCK_INPUTS there are unnecessary).

Thanks for the info.  I was not aware of that.  It would be much
cleaner and safer than adding BLOCK_INPUT in an ad-hoc way.

I tried SYNC_INPUT with the X11 build on Mac OS X, which can use SIGIO
for notification of input events.  It worked fine, but I noticed that
we couldn't quit a synchronous subprocess if it produced no output.
For example, `M-! yes RET' can be quit but `M-! sleep 10 RET' cannot.

I also did some experiments with the Carbon build in conjunction with
the following patch so that malloc may not occur in an asynchronous
way also in ``SIGALRM systems''.  It seems to work fine as long as I
tested, except the synchronous process issue above.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Index: src/keyboard.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/keyboard.c,v
retrieving revision 1.791
diff -c -r1.791 keyboard.c
*** src/keyboard.c	20 Aug 2004 10:34:12 -0000	1.791
--- src/keyboard.c	4 Sep 2004 08:49:50 -0000
***************
*** 2097,2103 ****
--- 2097,2107 ----
       struct atimer *timer;
  {
    if (poll_suppress_count == 0)
+ #ifdef SYNC_INPUT
+     interrupt_input_pending = 1;
+ #else
      poll_for_input_1 ();
+ #endif
  }
  
  #endif /* POLL_FOR_INPUT */

Re: BLOCK_INPUT on Mac OS X

[Stefan]

> I tried SYNC_INPUT with the X11 build on Mac OS X, which can use SIGIO
> for notification of input events.  It worked fine, but I noticed that
> we couldn't quit a synchronous subprocess if it produced no output.
> For example, `M-! yes RET' can be quit but `M-! sleep 10 RET' cannot.

Can you try and figure out where in the C code is Emacs looping?
A well placed QUIT in there should do the trick.


        Stefan

Re: BLOCK_INPUT on Mac OS X

[YAMAMOTO Mitsuharu]

>>>>> On 06 Sep 2004 12:17:45 -0400, Stefan <monnier@iro.umontreal.ca> said:

> Can you try and figure out where in the C code is Emacs looping?  A
> well placed QUIT in there should do the trick.

It seems to be looping in `emacs_read' called from `Fcall_process'.
Here's the backtrace.  I believe this can also be reproducible in
other "SIGIO systems".

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

(gdb) bt
#0  0x9000eac4 in read ()
#1  0x00139e90 in emacs_read (fildes=55, buf=0xbfff9d20 "", nbyte=16384) at /Users/mituharu/src/cvs/emacs/src/sysdep.c:3260
#2  0x002342f4 in Fcall_process (nargs=6, args=0xbfffe7dc) at /Users/mituharu/src/cvs/emacs/src/callproc.c:766
#3  0x002352c0 in Fcall_process_region (nargs=6, args=0xbfffe7dc) at /Users/mituharu/src/cvs/emacs/src/callproc.c:1146
#4  0x001ceb74 in Ffuncall (nargs=9, args=0xbfffe7d0) at /Users/mituharu/src/cvs/emacs/src/eval.c:2717
#5  0x00220900 in Fbyte_code (bytestr=3070347, vector=3070660, maxdepth=72) at /Users/mituharu/src/cvs/emacs/src/bytecode.c:689
#6  0x001cf848 in funcall_lambda (fun=3070252, nargs=6, arg_vector=0xbfffea54) at /Users/mituharu/src/cvs/emacs/src/eval.c:2923
#7  0x001cef94 in Ffuncall (nargs=7, args=0xbfffea50) at /Users/mituharu/src/cvs/emacs/src/eval.c:2784
#8  0x00220900 in Fbyte_code (bytestr=3068307, vector=3068612, maxdepth=64) at /Users/mituharu/src/cvs/emacs/src/bytecode.c:689
#9  0x001cf848 in funcall_lambda (fun=3068244, nargs=3, arg_vector=0xbfffecc4) at /Users/mituharu/src/cvs/emacs/src/eval.c:2923
#10 0x001cef94 in Ffuncall (nargs=4, args=0xbfffecc0) at /Users/mituharu/src/cvs/emacs/src/eval.c:2784
#11 0x001cdd60 in Fapply (nargs=2, args=0xbfffed88) at /Users/mituharu/src/cvs/emacs/src/eval.c:2241
#12 0x001ce460 in apply1 (fn=58798457, arg=17411981) at /Users/mituharu/src/cvs/emacs/src/eval.c:2494
#13 0x001c6464 in Fcall_interactively (function=58798457, record_flag=58721281, keys=34606228) at /Users/mituharu/src/cvs/emacs/src/callint.c:407
#14 0x00128038 in Fcommand_execute (cmd=58798457, record_flag=58721281, keys=58721281, special=58721281) at /Users/mituharu/src/cvs/emacs/src/keyboard.c:9725
#15 0x00114684 in command_loop_1 () at /Users/mituharu/src/cvs/emacs/src/keyboard.c:1776
#16 0x001cb64c in internal_condition_case (bfun=0x112640 <command_loop_1>, handlers=58766377, hfun=0x111cd8 <cmd_error>) at /Users/mituharu/src/cvs/emacs/src/eval.c:1346
#17 0x001122d4 in command_loop_2 () at /Users/mituharu/src/cvs/emacs/src/keyboard.c:1306
#18 0x001caec0 in internal_catch (tag=58761641, func=0x112294 <command_loop_2>, arg=58721281) at /Users/mituharu/src/cvs/emacs/src/eval.c:1107
#19 0x0011223c in command_loop () at /Users/mituharu/src/cvs/emacs/src/keyboard.c:1285
#20 0x00111768 in recursive_edit_1 () at /Users/mituharu/src/cvs/emacs/src/keyboard.c:978
#21 0x001119f4 in Frecursive_edit () at /Users/mituharu/src/cvs/emacs/src/keyboard.c:1039
#22 0x0010f6a8 in main (argc=3, argv=0xbffffd84) at /Users/mituharu/src/cvs/emacs/src/emacs.c:1687
(gdb) list
788	#ifdef VMS
789	     char **envp;
790	#endif
791	{
792	#if GC_MARK_STACK
793	  Lisp_Object dummy;
794	#endif
795	  char stack_bottom_variable;
796	  int do_initial_setlocale;
797	  int skip_args = 0;
(gdb) up
#1  0x00139e90 in emacs_read (fildes=55, buf=0xbfff9d20 "", nbyte=16384) at /Users/mituharu/src/cvs/emacs/src/sysdep.c:3260
3260	  while ((rtnval = read (fildes, buf, nbyte)) == -1
(gdb) list
3255	     char *buf;
3256	     unsigned int nbyte;
3257	{
3258	  register int rtnval;
3259	
3260	  while ((rtnval = read (fildes, buf, nbyte)) == -1
3261		 && (errno == EINTR));
3262	  return (rtnval);
3263	}
3264	
(gdb) up
#2  0x002342f4 in Fcall_process (nargs=6, args=0xbfffe7dc) at /Users/mituharu/src/cvs/emacs/src/callproc.c:766
766		    int this_read = emacs_read (fd[0], bufptr + nread,
(gdb) list
761		   of the buffer size we have.  But don't read
762		   less than 1024--save that for the next bufferful.  */
763		nread = carryover;
764		while (nread < bufsize - 1024)
765		  {
766		    int this_read = emacs_read (fd[0], bufptr + nread,
767						bufsize - nread);
768	
769		    if (this_read < 0)
770		      goto give_up;
(gdb) p interrupt_input_pending 
$1 = 1
(gdb) p Vquit_flag
$2 = 58721281
(gdb) p Vinhibit_quit
$3 = 58721281
(gdb) p Qnil
$4 = 58721281
(gdb) quit
The program is running.  Exit anyway? (y or n) y

Debugger finished

Re: BLOCK_INPUT on Mac OS X

[Stefan]

>> Can you try and figure out where in the C code is Emacs looping?  A
>> well placed QUIT in there should do the trick.

> It seems to be looping in `emacs_read' called from `Fcall_process'.
> Here's the backtrace.  I believe this can also be reproducible in
> other "SIGIO systems".

Can you try the patch below?
Beware, IIRC, if emacs_read is ever called somewhere where BLOCK_INPUT is
set, this could introduce other problems.


        Stefan


--- orig/src/sysdep.c
+++ mod/src/sysdep.c
@@ -1,6 +1,6 @@
 /* Interfaces to system-dependent kernel and library entries.
-   Copyright (C) 1985, 86,87,88,93,94,95,1999,2000,01,2003
-   Free Software Foundation, Inc.
+   Copyright (C) 1985, 1986, 1987, 1988, 1993, 1994, 1995, 1999,
+     2000, 2001, 2003, 2004  Free Software Foundation, Inc.
 
 This file is part of GNU Emacs.
 
@@ -3258,7 +3258,8 @@
   register int rtnval;
 
   while ((rtnval = read (fildes, buf, nbyte)) == -1
-	 && (errno == EINTR));
+	 && (errno == EINTR))
+    QUIT;
   return (rtnval);
 }

Re: BLOCK_INPUT on Mac OS X

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 07 Sep 2004 08:27:59 -0400, Stefan <monnier@iro.umontreal.ca> said:

>> It seems to be looping in `emacs_read' called from `Fcall_process'.
>> Here's the backtrace.  I believe this can also be reproducible in
>> other "SIGIO systems".

> Can you try the patch below?  Beware, IIRC, if emacs_read is ever
> called somewhere where BLOCK_INPUT is set, this could introduce
> other problems.

It didn't work for me on both Mac OS X and GNU/Linux.  Actually,
`read', which has not received any data, is silently restarted after
processing a signal because:

  1. That's a default behavior in BSD systems. (Mac OS X)
  2. SA_RESTART is set in `sys_signal' if POSIX_SIGNALS is
     defined. (GNU/Linux)

If SA_RESTART is not set in `sys_signal', synchronous processes can be
quit on GNU/Linux (with your patch).  But that breaks "Emacs mostly
works better with restartable system services" (from a comment in
`sys_signal').  

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: BLOCK_INPUT on Mac OS X

[Stefan Monnier]

>> Can you try the patch below?  Beware, IIRC, if emacs_read is ever
>> called somewhere where BLOCK_INPUT is set, this could introduce
>> other problems.

> It didn't work for me on both Mac OS X and GNU/Linux.  Actually,
> `read', which has not received any data, is silently restarted after
> processing a signal because:

>   1. That's a default behavior in BSD systems. (Mac OS X)
>   2. SA_RESTART is set in `sys_signal' if POSIX_SIGNALS is
>      defined. (GNU/Linux)

Duh!  I guess we'd need to unset SA_RESTART when we use SYNC_INPUT.

> If SA_RESTART is not set in `sys_signal', synchronous processes can be
> quit on GNU/Linux (with your patch).  But that breaks "Emacs mostly
> works better with restartable system services" (from a comment in
> `sys_signal').

I'm curious what this comment refers to.  Anybody remembers what are the
problems with non-restartable system services?  Is it possible that those
problems are not applicable in the case where we use SYNC_INPUT
(i.e. when we keep signal handlers to the strict minimum)?


        Stefan

Re: BLOCK_INPUT on Mac OS X

[Dr. Carsten Bormann]

Two millicenturies ago,
Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>>> Can you try the patch below?  Beware, IIRC, if emacs_read is ever
>>> called somewhere where BLOCK_INPUT is set, this could introduce
>>> other problems.
>
>> It didn't work for me on both Mac OS X and GNU/Linux.  Actually,
>> `read', which has not received any data, is silently restarted after
>> processing a signal because:
>
>>   1. That's a default behavior in BSD systems. (Mac OS X)
>>   2. SA_RESTART is set in `sys_signal' if POSIX_SIGNALS is
>>      defined. (GNU/Linux)
>
> Duh!  I guess we'd need to unset SA_RESTART when we use SYNC_INPUT.
>
>> If SA_RESTART is not set in `sys_signal', synchronous processes can be
>> quit on GNU/Linux (with your patch).  But that breaks "Emacs mostly
>> works better with restartable system services" (from a comment in
>> `sys_signal').
>
> I'm curious what this comment refers to.  Anybody remembers what are the
> problems with non-restartable system services?  Is it possible that those
> problems are not applicable in the case where we use SYNC_INPUT
> (i.e. when we keep signal handlers to the strict minimum)?

As you are probably aware, restartable system calls were added to BSD
to enable job control -- without restartable system calls, suspending
a process (which is mapped to a signal internally) might mean that a
write(2) was cut short and nothing in the (predominant sloppy) code
would notice.  Also, most programs just weren't prepared to handle
EINTR returns from read(2).  (This is from memory, about a quarter
century ago...)

I don't know how well the Emacs code checks the return values on
system calls like write.  Of course, I'd assume, very very well.

So I think we should pick up the somewhat stale discussion and do
what's necessary to arrive at a stable Emacs.  Why don't we just set
BROKEN_SA_RESTART with SYNC_INPUT?

Gruesse, Carsten

Re: BLOCK_INPUT on Mac OS X

[Stefan Monnier]

> So I think we should pick up the somewhat stale discussion and do
> what's necessary to arrive at a stable Emacs.  Why don't we just set
> BROKEN_SA_RESTART with SYNC_INPUT?

That's indeed what I'm using here.  Thanks for reminding me, I've installed
a few corresponding changes on the trunk.


        Stefan

Re: BLOCK_INPUT on Mac OS X

[YAMAMOTO Mitsuharu]

>>>>> On Thu, 25 Nov 2004 14:58:22 -0500, Stefan Monnier <monnier@iro.umontreal.ca> said:

> That's indeed what I'm using here.  Thanks for reminding me, I've
> installed a few corresponding changes on the trunk.

To benefit from this on Mac OS X, we need to define POSIX_SIGNALS in
src/s/darwin.h.   In src/s/freebsd.h, there is a comment as:

  /* Use sigprocmask(2) and friends instead of sigblock(2); the man page
     of sigblock says it is obsolete.  */

  #define POSIX_SIGNALS  	  1

and the sigblock(2) man page in Mac OS X says

    This interface is made obsolete by sigprocmask(2).

So that's probably OK, and actually worked fine for me.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

POSIX_SIGNALS (was: BLOCK_INPUT on Mac OS X)

[Stefan Monnier]

>> That's indeed what I'm using here.  Thanks for reminding me, I've
>> installed a few corresponding changes on the trunk.

> To benefit from this on Mac OS X, we need to define POSIX_SIGNALS in
> src/s/darwin.h.   In src/s/freebsd.h, there is a comment as:

Hmm... shouldn't this be defined automatically by the configure script?


        Stefan

Re: POSIX_SIGNALS (was: BLOCK_INPUT on Mac OS X)

[YAMAMOTO Mitsuharu]

>>>>> On Fri, 26 Nov 2004 09:24:58 -0500, Stefan Monnier <monnier@iro.umontreal.ca> said:

>> To benefit from this on Mac OS X, we need to define POSIX_SIGNALS
>> in src/s/darwin.h.  In src/s/freebsd.h, there is a comment as:

> Hmm... shouldn't this be defined automatically by the configure
> script?

I agree with you.

For using SYNC_INPUT properly in Carbon Emacs, we need some more
changes.  Could you see the following patch (also appeared in
http://lists.gnu.org/archive/html/emacs-devel/2004-09/msg00094.html)
for the systems that use polling, and install if it is correct?

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp


Index: src/keyboard.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/keyboard.c,v
retrieving revision 1.791
diff -c -r1.791 keyboard.c
*** src/keyboard.c	20 Aug 2004 10:34:12 -0000	1.791
--- src/keyboard.c	4 Sep 2004 08:49:50 -0000
***************
*** 2097,2103 ****
--- 2097,2107 ----
       struct atimer *timer;
  {
    if (poll_suppress_count == 0)
+ #ifdef SYNC_INPUT
+     interrupt_input_pending = 1;
+ #else
      poll_for_input_1 ();
+ #endif
  }
  
  #endif /* POLL_FOR_INPUT */