[AC21.5] Avoid assert (l.6311 frame.c) from uninitialized glyph_cachels

APPROVE COMMIT 21.5 I don't think this patch is relevant to 21.4, and it certainly won't apply. Let me know if you want me to check more carefully, Vin. This patch fixes the assert described above by initializing (struct window).glyph_cachels everywhere (struct window).face_cachels is initialized. I believe that the assert was triggered by a redisplay caused by attempting to write to the echo area in the process of frame creation. I'm not sure how that can happen (the context where I observed it is a rapid series of make-frames by VM in creating a virtual folder with lots of source folders, which also triggered a suicide-by-SIGPIPE!), but I'm pretty sure that it was not possible to exist Fmake_frame() with the glyph_cachels before this patch. Nevertheless, the patch helps. :-/ I also removed some unneeded code and improved comments related to initialization of glyph_cachels. diff -r d8a11d5ebc9f -r 3fde0e346ad7 src/ChangeLog --- a/src/ChangeLog Fri Oct 28 23:52:26 2011 +0900 +++ b/src/ChangeLog Sat Oct 29 00:35:33 2011 +0900 ＠＠ -1,3 +1,16 ＠＠ +2011-10-29 Stephen J. Turnbull <stephen(a)xemacs.org&gt; + + Prevent assert at frame.c, l. 6311 by initializing glyph cachels. + + * frame.c (Fmake_frame): Ensure that reset_glyph_cachels gets called. + (setup_minibuffer_frame): Improve header comment. + + * redisplay.c (redisplay_window): Update comment. + Remove code checking for uninitialized face_cachels and glyph_cachels. + Can't happen in theory, and guarded by asserts in practice. + + * window.c (allocate_window): Update comment on reset_*_cachels. + 2011-10-09 Aidan Kehoe <kehoea(a)parhasard.net&gt; * fns.c (remassoc_no_quit): diff -r d8a11d5ebc9f -r 3fde0e346ad7 src/frame.c --- a/src/frame.c Fri Oct 28 23:52:26 2011 +0900 +++ b/src/frame.c Sat Oct 29 00:35:33 2011 +0900 ＠＠ -793,7 +793,8 ＠＠ f->minibuffer_window = Qnil; } -/* Make a frame containing only a minibuffer window. */ +/* Make a frame containing only a minibuffer window. + The minibuffer window is also the root window. */ static void setup_minibuffer_frame (struct frame *f) ＠＠ -957,8 +958,12 ＠＠ if (initialized && !DEVICE_STREAM_P (d)) { if (!NILP (f->minibuffer_window)) - reset_face_cachels (XWINDOW (f->minibuffer_window)); + { + reset_face_cachels (XWINDOW (f->minibuffer_window)); + reset_glyph_cachels (XWINDOW (f->minibuffer_window)); + } reset_face_cachels (XWINDOW (f->root_window)); + reset_glyph_cachels (XWINDOW (f->root_window)); } /* If no frames on this device formerly existed, say this is the diff -r d8a11d5ebc9f -r 3fde0e346ad7 src/redisplay.c --- a/src/redisplay.c Fri Oct 28 23:52:26 2011 +0900 +++ b/src/redisplay.c Sat Oct 29 00:35:33 2011 +0900 ＠＠ -6306,18 +6306,19 ＠＠ because we initialized them at startup and the only way to reduce their number is through calling reset_face_cachels() or reset_glyph_cachels(), which as a side effect sets up a number of - standard entries */ + standard entries + 2011-10-29 -- We were managing to hit the glyph_cachels assert in certain + contexts where VM was creating a lot of frames. I don't have a full + analysis, but I suspect that we were failing to setup the glyph_cachels + at about l. 961 of frame.c, and a message was being sent to the echo area + before the initialization was complete. This triggered a redisplay of + the minibuffer window (this part is confirmed), and thus this assert. */ assert (Dynarr_length (w->face_cachels)); assert (Dynarr_length (w->glyph_cachels)); /* If the buffer has changed we have to invalidate all of our face cache elements. */ if ((!echo_active && b != window_display_buffer (w)) -#if 0 - /* #### Delete this code sometime later than 2-1-10 when we're sure it's - not needed */ - || !Dynarr_length (w->face_cachels) -#endif || f->faces_changed) reset_face_cachels (w); else ＠＠ -6327,11 +6328,6 ＠＠ the cache purely because glyphs have changed - this is now handled by the dirty flag.*/ if ((!echo_active && b != window_display_buffer (w)) -#if 0 - /* #### Delete this code sometime later than 2-1-10 when we're sure it's - not needed */ - || !Dynarr_length (w->glyph_cachels) -#endif || f->faces_changed) reset_glyph_cachels (w); else diff -r d8a11d5ebc9f -r 3fde0e346ad7 src/window.c --- a/src/window.c Fri Oct 28 23:52:26 2011 +0900 +++ b/src/window.c Sat Oct 29 00:35:33 2011 +0900 ＠＠ -383,7 +383,10 ＠＠ All callers of allocate_window should therefore call reset_face_cachels on the created window. We can't do it here because the window must have its frame pointer set or - reset_face_cachels will fail. */ + reset_face_cachels will fail. + A similar requirement holds for reset_glyph_cachels. We *could* do + that here (there's no reference to the frame pointer in that function), + but we may as well have the same discipline. */ Lisp_Object allocate_window (void) { _______________________________________________ XEmacs-Patches mailing list XEmacs-Patches(a)xemacs.org http://lists.xemacs.org/mailman/listinfo/xemacs-patches