From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: "Richard M. Stallman" Newsgroups: gmane.emacs.devel Subject: Re: allocate_string_data memory corruption Date: Sat, 21 Jan 2006 22:57:50 -0500 Message-ID: References: <87vewha2zl.fsf@stupidchicken.com> <87zmlq6w62.fsf-monnier+emacs@gnu.org> <87vewda3od.fsf@stupidchicken.com> Reply-To: rms@gnu.org NNTP-Posting-Host: main.gmane.org Content-Type: text/plain; charset=ISO-8859-15 X-Trace: sea.gmane.org 1137902495 18365 80.91.229.2 (22 Jan 2006 04:01:35 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Sun, 22 Jan 2006 04:01:35 +0000 (UTC) Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Jan 22 05:01:33 2006 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by ciao.gmane.org with esmtp (Exim 4.43) id 1F0WQ0-0006G9-SZ for ged-emacs-devel@m.gmane.org; Sun, 22 Jan 2006 05:01:33 +0100 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1F0WSZ-00087m-QN for ged-emacs-devel@m.gmane.org; Sat, 21 Jan 2006 23:04:11 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1F0WSJ-000866-6z for emacs-devel@gnu.org; Sat, 21 Jan 2006 23:03:55 -0500 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1F0WSI-00085W-KQ for emacs-devel@gnu.org; Sat, 21 Jan 2006 23:03:54 -0500 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1F0WSI-00085P-I5 for emacs-devel@gnu.org; Sat, 21 Jan 2006 23:03:54 -0500 Original-Received: from [199.232.76.164] (helo=fencepost.gnu.org) by monty-python.gnu.org with esmtp (Exim 4.34) id 1F0WWn-0005FC-CX for emacs-devel@gnu.org; Sat, 21 Jan 2006 23:08:33 -0500 Original-Received: from rms by fencepost.gnu.org with local (Exim 4.34) id 1F0WMQ-0006TV-BO; Sat, 21 Jan 2006 22:57:50 -0500 Original-To: Chong Yidong In-reply-to: <87vewda3od.fsf@stupidchicken.com> (message from Chong Yidong on Sat, 21 Jan 2006 12:31:14 -0500) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:49383 Archived-At: How do we get around this? Is it OK for x_catch_errors to return without doing anything, if it is called in a signal handler? I don't think so. Catching errors is the only way to recover from certain errors that the X server reports. And, as I've asked previously, do you see any disadvantage to simply using BLOCK_INPUT in the string/cons allocation functions? The reason I'd rather not do it is simply that it is a big change in the design paradigm that has been followed thus far. It is safer to keep the paradigm unchanged, and fix whatever place fails to follow it. After the release, we could look at changing this paradigm, perhaps to use SYNC_INPUT. One way to fix it now is to make a different function, which uses preallocated data, for use in a signal handler. Does this patch fix the problem? *** xterm.c 19 Jan 2006 12:29:03 -0500 1.891 --- xterm.c 21 Jan 2006 16:30:44 -0500 *************** *** 336,341 **** --- 336,347 ---- void x_wm_set_window_state P_ ((struct frame *, int)); void x_wm_set_icon_pixmap P_ ((struct frame *, int)); void x_initialize P_ ((void)); + + static int x_signal_catch_errors P_ ((Display *)); + static void x_signal_uncatch_errors P_ ((int)); + static int x_signal_had_errors_p P_ (()); + static Lisp_Object x_signal_catch_errors_unwind P_ (()); + static void x_font_min_bounds P_ ((XFontStruct *, int *, int *)); static int x_compute_min_glyph_bounds P_ ((struct frame *)); static void x_update_end P_ ((struct frame *)); *************** *** 3725,3731 **** structure is changing at the same time this function is running. So at least we must not crash from them. */ ! count = x_catch_errors (FRAME_X_DISPLAY (*fp)); if (FRAME_X_DISPLAY_INFO (*fp)->grabbed && last_mouse_frame && FRAME_LIVE_P (last_mouse_frame)) --- 3731,3737 ---- structure is changing at the same time this function is running. So at least we must not crash from them. */ ! count = x_signal_catch_errors (FRAME_X_DISPLAY (*fp)); if (FRAME_X_DISPLAY_INFO (*fp)->grabbed && last_mouse_frame && FRAME_LIVE_P (last_mouse_frame)) *************** *** 3791,3800 **** #endif /* USE_X_TOOLKIT */ } ! if (x_had_errors_p (FRAME_X_DISPLAY (*fp))) f1 = 0; ! x_uncatch_errors (FRAME_X_DISPLAY (*fp), count); /* If not, is it one of our scroll bars? */ if (! f1) --- 3797,3806 ---- #endif /* USE_X_TOOLKIT */ } ! if (x_signal_had_errors_p ()) f1 = 0; ! x_signal_uncatch_errors (count); /* If not, is it one of our scroll bars? */ if (! f1) *************** *** 5711,5717 **** Display *d = event.xclient.display; /* Catch and ignore errors, in case window has been iconified by a window manager such as GWM. */ ! int count = x_catch_errors (d); XSetInputFocus (d, event.xclient.window, /* The ICCCM says this is the only valid choice. */ --- 5717,5723 ---- Display *d = event.xclient.display; /* Catch and ignore errors, in case window has been iconified by a window manager such as GWM. */ ! int count = x_signal_catch_errors (d); XSetInputFocus (d, event.xclient.window, /* The ICCCM says this is the only valid choice. */ *************** *** 5720,5726 **** /* This is needed to detect the error if there is an error. */ XSync (d, False); ! x_uncatch_errors (d, count); } /* Not certain about handling scroll bars here */ #endif /* 0 */ --- 5726,5732 ---- /* This is needed to detect the error if there is an error. */ XSync (d, False); ! x_signal_uncatch_errors (count); } /* Not certain about handling scroll bars here */ #endif /* 0 */ *************** *** 7591,7596 **** --- 7597,7667 ---- #endif /* ! 0 */ + Display *x_signal_catch_errors_dpy; + + Lisp_Object x_signal_error_message_string; + + static int + x_signal_catch_errors (dpy) + Display *dpy; + { + int count = SPECPDL_INDEX (); + + /* Make sure any errors from previous requests have been dealt with. */ + XSync (dpy, False); + + x_signal_catch_errors_dpy = dpy; + record_unwind_protect (x_signal_catch_errors_unwind, Qnil); + + SSET (x_signal_error_message_string, 0, 0); + + return count; + } + + /* Unbind the binding that we made to check for X errors. */ + + static Lisp_Object + x_signal_catch_errors_unwind (old_val) + Lisp_Object old_val; + { + /* The display may have been closed before this function is called. + Check if it is still open before calling XSync. */ + if (x_display_info_for_display (x_signal_catch_errors_dpy) != 0) + { + BLOCK_INPUT; + XSync (x_signal_catch_errors_dpy, False); + UNBLOCK_INPUT; + } + + return Qnil; + } + + /* Nonzero if we had any X protocol errors + since we did x_signal_catch_errors. */ + + static int + x_signal_had_errors_p () + { + Display *dpy = x_signal_catch_errors_dpy; + + /* Make sure to catch any errors incurred so far. */ + XSync (dpy, False); + + return SREF (x_signal_error_message_string, 0) != 0; + } + + /* Stop catching X protocol errors and let them make Emacs die. + DPY should be the display that was passed to x_catch_errors. + COUNT should be the value that was returned by + the corresponding call to x_catch_errors. */ + + static void + x_signal_uncatch_errors (count) + int count; + { + unbind_to (count, Qnil); + } + /* Handle SIGPIPE, which can happen when the connection to a server simply goes away. SIGPIPE is handled by x_connection_signal. Don't need to do anything, because the write which caused the *************** *** 10837,10842 **** --- 10908,10916 ---- staticpro (&last_mouse_press_frame); last_mouse_press_frame = Qnil; + + staticpro (&x_signal_error_message_string); + x_signal_error_message_string = make_uninit_string (X_ERROR_MESSAGE_SIZE); DEFVAR_BOOL ("x-use-underline-position-properties", &x_use_underline_position_properties,