changeset 1313:671b65f2b075

[xemacs-hg @ 2003-02-20 01:12:25 by ben] patch for crashes when deleting frames eval.c: Don't check_quit() unless we're unbinding a real Lisp `unwind-protect' since check_quit() does lots of weird things and not all callers are prepared for it. frame.c: Make absolutely sure there is no quit checking while we are in a "critical section" during frame deletion.
author ben
date Thu, 20 Feb 2003 01:12:26 +0000
parents 22741407fc1f
children 15a91d7ae2d1
files src/ChangeLog src/eval.c src/frame.c
diffstat 3 files changed, 53 insertions(+), 11 deletions(-) [+]
line wrap: on
line diff
--- a/src/ChangeLog	Wed Feb 19 22:52:37 2003 +0000
+++ b/src/ChangeLog	Thu Feb 20 01:12:26 2003 +0000
@@ -1,3 +1,14 @@
+2003-02-19  Ben Wing  <ben@xemacs.org>
+
+	* eval.c (unbind_to_hairy):
+	Don't check_quit() unless we're unbinding a real Lisp
+	`unwind-protect' since check_quit() does lots of weird things and
+	not all callers are prepared for it.
+	* frame.c:
+	* frame.c (delete_frame_internal):
+	Make absolutely sure there is no quit checking while we are in a
+	"critical section" during frame deletion.
+
 2003-02-16  Steve Youngs  <youngs@xemacs.org>
 
 	* XEmacs 21.5.11 "cabbage" is released.
--- a/src/eval.c	Wed Feb 19 22:52:37 2003 +0000
+++ b/src/eval.c	Thu Feb 20 01:12:26 2003 +0000
@@ -5930,25 +5930,47 @@
 void
 unbind_to_hairy (int count)
 {
-  Lisp_Object oquit;
-
   ++specpdl_ptr;
   ++specpdl_depth_counter;
 
-  /* Allow QUIT within unwind-protect routines, but defer any existing QUIT
-     until afterwards. */
-  check_quit (); /* make Vquit_flag accurate */
-  oquit = Vquit_flag;
-  Vquit_flag = Qnil;
-
   while (specpdl_depth_counter != count)
     {
+      Lisp_Object oquit = Qunbound;
+
+      /* Do this check BEFORE decrementing the values below, because once
+	 they're decremented, GC protection is lost on
+	 specpdl_ptr->old_value. */
+      if (specpdl_ptr->func == Fprogn)
+	{
+	  /* Allow QUIT within unwind-protect routines, but defer any
+	     existing QUIT until afterwards.  Only do this, however, for
+	     unwind-protects established by Lisp code, not by C code
+	     (e.g. free_opaque_ptr() or something), because the act of
+	     checking for QUIT can cause all sorts of weird things to
+	     happen, since it churns the event loop -- redisplay, running
+	     Lisp, etc.  Code should not have to worry about this just
+	     because of establishing an unwind-protect. */
+	  check_quit (); /* make Vquit_flag accurate */
+	  oquit = Vquit_flag;
+	  Vquit_flag = Qnil;
+	}
+
       --specpdl_ptr;
       --specpdl_depth_counter;
 
+      /* #### At this point, there is no GC protection on old_value.  This
+	 could be a real problem, depending on what unwind-protect function
+	 is called.  It looks like it just so happens that the ones
+	 actually called don't have a problem with this, e.g. Fprogn.  But
+	 we should look into fixing this. (Many unwind-protect functions
+	 free values.  Is it a problem if freed values are
+	 GC-protected?) */
       if (specpdl_ptr->func != 0)
-        /* An unwind-protect */
-	(*specpdl_ptr->func) (specpdl_ptr->old_value);
+	{
+	  /* An unwind-protect */
+	  (*specpdl_ptr->func) (specpdl_ptr->old_value);
+	}
+	  
       else
 	{
 	  /* We checked symbol for validity when we specbound it,
@@ -5979,8 +6001,10 @@
         }
 #endif
 #endif
+
+      if (!UNBOUNDP (oquit))
+	Vquit_flag = oquit;
     }
-  Vquit_flag = oquit;
   check_specbind_stack_sanity ();
 }
 
--- a/src/frame.c	Wed Feb 19 22:52:37 2003 +0000
+++ b/src/frame.c	Thu Feb 20 01:12:26 2003 +0000
@@ -1459,6 +1459,7 @@
   Lisp_Object device;
   Lisp_Object console;
   struct gcpro gcpro1;
+  int depth;
 
   /* OK to delete an already deleted frame. */
   if (!FRAME_LIVE_P (f))
@@ -1682,6 +1683,10 @@
 
   /* After this point, no errors must be allowed to occur. */
 
+  /* Checking for QUIT can run all sorts of weird code and may be deadly
+     so don't let it happen. */
+  depth = begin_dont_check_for_quit ();
+
 #ifdef HAVE_MENUBARS
   free_frame_menubars (f);
 #endif
@@ -1822,6 +1827,8 @@
 
   note_object_deleted (frame);
 
+  unbind_to (depth);
+
   UNGCPRO;
 }