Mac OS X - Hang / C-g problem patch

* Mac OS X - Hang / C-g problem patch
@ 2002-12-04 18:06 Steven Tamm
  2002-12-05  6:48 ` Andrew Choi
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Tamm @ 2002-12-04 18:06 UTC (permalink / raw)


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

I just found some free time on my hands and got back around to working 
on the problem where in the carbon gui, C-g does not interrupt 
synchronous subprocesses.  This caused much consternation and after 
checking with some people at apple, there was no easy way.  The 
alternatives were to finish my Jaguar/MT UI patch (which I won't go 
into), or to make all blocking reads be in a separate thread.

This patch overrides read to call sys_read.  sys_read will call 
darwin's read if inhibit_window_system, noninteractive, if the file 
descriptor is not FIFO, or if the file descriptor is non-blocking.  So 
it only differs when blocks with pipes and sockets.  It works by 
spawning a worker thread that handles the read while waiting on a 
MPQueue (chose this instead of pthread_cond for simplicity).  When a 
potentially hanging read is started, it posts a message to the worker 
threads queue and then blocks for an event coming back from the worker 
thread and also KeyUp events.  If the worker thread event comes back to 
the main thread, it returns like normal.  If a keyup event comes in, it 
is tested to see if it is the quit_char, and if so, the worker thread 
is signalled and Fsignal is called to unroll the call.  It seems to 
work and hopefully it plugs a large hole.

Issues:
1.  Uses fstat and fcntl way too often.  These calls aren't 
extraordinarily expensive and I didn't notice a difference, but your 
mileage may vary.
2.  Traps keyUp instead of keyDown.  This behavior is broken because it 
makes emacs think C-g has been called twice.  But it makes things much 
simpler.  Otherwise, the main thread would have to dispatch all events 
to the kbd_buffer, which with the current event handling is an issue.  
If we switched to Carbon Event Handers, this would be a non-issue (in 
fact there would be less code).
3.  Only works when quit_char is C-g.  There is docs on how to fix it, 
but I didn't want to wait.
4.  Only works on darwin (due to use of pthread_kill).  I probably 
should have put in an ifdef...

Not tested:
1.  Sending random signals to the emacs process to see if the reads 
continue.  There may need to be some checking in signal handlers 
elsewhere to cause the secondary thread to break.
2.  Memory leaks.

I would appreciate any/all Mac OS X developers to try the patch and let 
me know how it goes.

-Steven

[-- Attachment #2: carbon_quit_char.patch --]
[-- Type: application/octet-stream, Size: 8777 bytes --]

Index: src/sysdep.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/sysdep.c,v
retrieving revision 1.245
diff -u -d -b -w -r1.245 sysdep.c

--- src/sysdep.c	22 Nov 2002 12:22:43 -0000	1.245
+++ src/sysdep.c	4 Dec 2002 17:52:59 -0000
@@ -69,6 +69,10 @@
 #endif
 #endif /* not WINDOWSNT */
 
+#ifdef HAVE_CARBON
+#define read sys_read
+#endif
+
 /* Does anyone other than VMS need this? */
 #ifndef fwrite
 #define sys_fwrite fwrite
Index: src/macterm.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/macterm.c,v
retrieving revision 1.26
diff -u -d -b -w -r1.26 macterm.c
--- src/macterm.c	2 Dec 2002 17:13:45 -0000	1.26
+++ src/macterm.c	4 Dec 2002 17:53:02 -0000
@@ -12630,7 +12630,7 @@
 				expected
 				? TicksToEventTime (app_sleep_time)
 				: 0,
-				true, &eventRef);
+				kEventRemoveFromQueue, &eventRef);
   if (!rneResult)
     {
       /* Handle new events */
@@ -13446,6 +13446,60 @@
 \f
 #ifdef MAC_OSX
 void
+mac_check_bundle()
+{
+  extern int inhibit_window_system;
+  extern int noninteractive;
+  CFBundleRef appsBundle;
+  pid_t child;
+
+  /* No need to test if already -nw*/
+  if (inhibit_window_system || noninteractive)
+    return;
+
+  appsBundle = CFBundleGetMainBundle();
+  if (appsBundle != NULL)
+    {
+      CFStringRef cfBI = CFSTR("CFBundleIdentifier");
+      CFTypeRef res = CFBundleGetValueForInfoDictionaryKey(appsBundle, cfBI);
+      /* We found the bundle identifier, now we know we are valid. */
+      if (res != NULL)
+	{
+	  CFRelease(res);
+	  return;
+	}
+    }
+
+  /* Now we must find the "bundled" executable and exec it */
+  child = fork();
+  if (child < 0) 
+    {
+      /* If there is an error, try to keep going inside the terminal */
+      inhibit_window_system = 1;
+    }
+  if (!child) 
+    {
+      int i;
+      extern char **initial_argv;
+      extern int initial_argc;
+      char *newApp = "/Applications/Emacs.app/Contents/MacOS/Emacs";
+      register const unsigned char **new_argv =
+	(const unsigned char **) alloca ((initial_argc+1) * sizeof(char*));
+      new_argv[0] = newApp;
+      for (i = 1; i < initial_argc; i++) 
+	new_argv[i] = initial_argv[i];
+      new_argv[initial_argc]=NULL;  /* Will point to last entry*/
+      execv(newApp, new_argv);
+      _exit(-1);
+    } 
+  else
+    {
+      /* Started successfully */
+      exit(0);
+    }
+}
+
+void
 MakeMeTheFrontProcess ()
 {
   ProcessSerialNumber psn;
@@ -13454,6 +13508,195 @@
   err = GetCurrentProcess (&psn);
   if (err == noErr)
     (void) SetFrontProcess (&psn);
+}
+
+/* Fix C-g hang in emacs by placing blocking IO in separate thread.  */
+#define kEventClassEmacs 'EMAx'
+#define kEventEmacsReadDone 'read'
+#define kEventParamErrorNo 'errn'
+MPQueueID reader_queue = NULL;
+pthread_t mac_reader_thread = NULL;
+
+void 
+mac_reader_loop (void *arg)
+{
+  sigset_t sigs;
+  sigfillset(&sigs);
+  sigdelset(&sigs, SIGINT);
+  sigdelset(&sigs, SIGURG);
+  pthread_sigmask(SIG_SETMASK, &sigs, NULL);
+  while (1) 
+    {
+      int fildes;
+      char *buf;
+      unsigned int nbyte;
+      ssize_t result;
+      int curerr = 0;
+      EventRef event;
+
+      MPWaitOnQueue(reader_queue, (void**)&fildes, (void**)&buf, (void**)&nbyte, kDurationForever);
+      result = read(fildes, buf, nbyte);   
+      if (result < 0)
+	curerr = errno;
+
+      CreateEvent(NULL, kEventClassEmacs, kEventEmacsReadDone,
+		  0, kEventAttributeUserEvent, &event);
+      SetEventParameter(event, kEventParamDirectObject, typeUInt32, 
+			sizeof(UInt32), &result);
+      SetEventParameter(event, kEventParamErrorNo, typeUInt32, 
+			sizeof(UInt32), &curerr);
+      PostEventToQueue(GetMainEventQueue(), event,
+		       kEventPriorityHigh);
+      ReleaseEvent(event);
+
+      /* Post any stolen events back to the event queue at once */
+    }
+}
+
+UInt32 mac_quit_char_keycode;
+UInt32 mac_quit_char_modifiers;
+#define kModifierMask (controlKey|cmdKey|optionKey|shiftKey)
+
+static OSStatus
+mac_register_quit_char()
+{
+  UInt32 modifiers = ctrl_modifier;
+  mac_quit_char_modifiers = 0;
+  mac_quit_char_keycode = 0x05;  
+  /* FIXME: if quit_char is not C-g, then we need to map from 
+     ascii to keyCode.  Look at <architecture/adb_kb_map.h> for details */
+  /*http://gemma.apple.com/techpubs/mac/Toolbox/Toolbox-40.html#MARKER-9-184*/
+  
+  /* Map modifiers */
+  if (modifiers & ctrl_modifier)  mac_quit_char_modifiers |= macCtrlKey;
+  if (modifiers & shift_modifier) mac_quit_char_modifiers |= macShiftKey;
+  if (modifiers & meta_modifier)  mac_quit_char_modifiers |= macMetaKey;
+  if (modifiers & alt_modifier)   mac_quit_char_modifiers |= macAltKey;
+}
+
+static void
+initialize_reader_thread()
+{
+  if (!mac_reader_thread)
+    {
+      int result;
+      mac_register_quit_char();
+      MPCreateQueue(&reader_queue);
+      if (!pthread_create(&mac_reader_thread, NULL, mac_reader_loop, NULL))
+	{
+	  /* For some reason pthread_create is working, but returning 200. */
+	}
+    }
+}
+
+static UInt32
+mac_blocked_io_call()
+{
+  UInt32 result;
+  EventRef event;
+  OSStatus err;
+
+  static EventTypeSpec mac_reader_events[] = {
+    {kEventClassKeyboard, kEventRawKeyUp}, /* See note below*/
+    {kEventClassEmacs, kEventEmacsReadDone}
+  };
+
+  /* This relies on the fact that keyUp events are ignored by emacs,
+   and are, in general, ignored by everything else.  This means we
+   can suck all the events off the queue and remain in a wait state.
+   Using keydown causes issues because you cannot leave the event on 
+   the queue, or this call will be constantly called with that event.
+   So to use keydown, you must either save the event for posting later
+   or generate the input_event here.  Generating the input_event is
+   probably better.
+
+   The switch to keyDown should be made to make the behavior uniform
+   across platforms, but this would require splitting up XTread_socket.
+   Hopefully, then the event handling will be split into a separate file.
+   Using standard carbon event handlers with the kbd_buffer_store_event
+   code would probably be best.
+  */
+  while (1) 
+    {
+      err = ReceiveNextEvent(2, mac_reader_events, kEventDurationForever,
+			     kEventRemoveFromQueue, &event);
+      if (err != noErr) 
+	fprintf(stderr, "Got Error in RNE: %d", err);
+      UInt32 eClass = GetEventClass(event);
+      if (eClass == kEventClassEmacs) 
+	{
+	  int threadErrno = 0;
+	  err = GetEventParameter (event, kEventParamDirectObject, typeUInt32,
+				   NULL, sizeof(UInt32), NULL, &result);
+	  if (err != noErr) 
+	    fprintf(stderr, "Got Error in RNE: %d", err);
+	  /* Need to get errno from read thread to main thread here. */
+	  err = GetEventParameter (event, kEventParamErrorNo, typeUInt32,
+				   NULL, sizeof(UInt32), NULL, &threadErrno);
+	  if (err != noErr) 
+	    fprintf(stderr, "Got Error in RNE: %d", err);
+	  errno = threadErrno;
+	  break;
+	}
+      else 
+	{
+	  UInt32 keyCode, modifiers;
+	  GetEventParameter(event, kEventParamKeyCode, typeUInt32, NULL,
+			    sizeof(UInt32), NULL, &keyCode);
+	  if (keyCode == mac_quit_char_keycode)
+	    {
+	      GetEventParameter(event, kEventParamKeyModifiers, typeUInt32,
+				NULL, sizeof(UInt32), NULL, &modifiers);
+	      if ((modifiers & kModifierMask) == mac_quit_char_modifiers) 
+		{
+		  /* Signal the read to stop */
+		  pthread_kill(mac_reader_thread, SIGURG); 
+
+		  /* Instead of signalling directly, possibly use 
+		     kbd_buffer_store_event with a quit_char event.  */
+		  Fsignal (Qquit, Qnil);		  
+		  /* NOTREACHED */
+		  result = -1;
+		}
+	    }
+	}
+    }
+  ReleaseEvent(event);
+  return result;
+}
+
+int
+sys_read (fildes, buf, nbyte)
+     int fildes;
+     char *buf;
+     unsigned int nbyte;
+{
+#if !USE_CARBON_EVENTS
+  return read(fildes, buf, nbyte);
+#endif
+
+  /* If not GUI, use default behavior */
+
+  if (inhibit_window_system || noninteractive)
+    return read(fildes, buf, nbyte);
+
+  /* Only use on sync threaded read on blocking pipes and sockets */
+  {
+    struct stat stat;
+    fstat(fildes, &stat);
+    if (!S_ISFIFO(stat.st_mode) || fcntl(fildes, F_GETFL, 0) & O_NONBLOCK)
+      return read(fildes, buf, nbyte);
+  }
+
+  initialize_reader_thread();
+
+  MPNotifyQueue(reader_queue, (void*)fildes, buf, (void*)nbyte);
+
+  {
+    int result;
+    result = mac_blocked_io_call();
+    return result;
+  }
 }
 #endif /* MAC_OSX */
 

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


P.S.  This patch also contains the mac_check_bundle code I was playing 
around with to get emacs from the command line to start the GUI.

^ permalink raw reply	[flat|nested] 11+ messages in thread