From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Alan Third Newsgroups: gmane.emacs.bugs Subject: bug#65843: 28.2; Too many iconified frames in .emacs.desktop -> crash (macOS) Date: Sat, 9 Sep 2023 20:53:45 +0100 Message-ID: References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="AFEQo+OQYIoInont" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="31057"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 65843@debbugs.gnu.org To: tanzer@gg32.com Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Sep 09 21:55:16 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qf432-0007td-88 for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 09 Sep 2023 21:55:16 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qf42n-0000LD-QH; Sat, 09 Sep 2023 15:55:01 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qf42l-0000Kq-Ku for bug-gnu-emacs@gnu.org; Sat, 09 Sep 2023 15:55:00 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qf42l-0006Ak-Ce for bug-gnu-emacs@gnu.org; Sat, 09 Sep 2023 15:54:59 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qf42o-0000k5-2Z for bug-gnu-emacs@gnu.org; Sat, 09 Sep 2023 15:55:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Alan Third Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 09 Sep 2023 19:55:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 65843 X-GNU-PR-Package: emacs Original-Received: via spool by 65843-submit@debbugs.gnu.org id=B65843.16942892432785 (code B ref 65843); Sat, 09 Sep 2023 19:55:02 +0000 Original-Received: (at 65843) by debbugs.gnu.org; 9 Sep 2023 19:54:03 +0000 Original-Received: from localhost ([127.0.0.1]:48516 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qf41p-0000ic-5x for submit@debbugs.gnu.org; Sat, 09 Sep 2023 15:54:03 -0400 Original-Received: from dane.soverin.net ([2a10:de80:1:4092:b9e9:229d:0:1]:50085) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qf41k-0000iH-9q for 65843@debbugs.gnu.org; Sat, 09 Sep 2023 15:54:00 -0400 Original-Received: from smtp.soverin.net (c04smtp-lb01.int.sover.in [10.10.4.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dane.soverin.net (Postfix) with ESMTPS id 4RjkF65mSczytv; Sat, 9 Sep 2023 19:53:46 +0000 (UTC) Original-Received: from smtp.soverin.net (smtp.soverin.net [10.10.4.100]) by soverin.net (Postfix) with ESMTPSA id 4RjkF60VGrzdR; Sat, 9 Sep 2023 19:53:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=idiocy.org; s=soverin; t=1694289226; bh=LRqQEOe7F8+VuxMT0kiOocgYdvNG4NraDJY1WWqtmFI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ouDP3N40dBq7TsIpdanKSaUnzCIzQd6Hn1lltGp3B3sktn5hAlC2Dm/SFSVXZaNDK pggeU3GVUOjxWcX8pRoGmg+ebJ0BdhtyC5ldRNdKpGnihZA/6p3qK7dTt+4PYj4NZq elWANkpk1HgKUWZKLG9txr0G+rJJ3XZQzFWaZBZhSFzWjQA0TOxPDex41wncn8yoTf 5cyX1m25q5BHez5Fx6FXQobFHTnz/jgCZNGGhhI4zw/A/+mI0aUagjPbAzx2kllRcw /9gsWeKtReJ7DUlKI90JImxfQSHS9p8sKOySe8KX6hUh5eX1aMkqDVZ7oxEZgvnhyI XlBoBqFQGA4Xg== Original-Received: from alan by faroe.holly.idiocy.org with local (Exim 4.96) (envelope-from ) id 1qf41Z-001ROT-0g; Sat, 09 Sep 2023 20:53:45 +0100 X-Soverin-Authenticated: true Mail-Followup-To: Alan Third , tanzer@gg32.com, 65843@debbugs.gnu.org Content-Disposition: inline In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:269897 Archived-At: --AFEQo+OQYIoInont Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sat, Sep 09, 2023 at 04:35:56PM -0000, Christian Tanzer via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote: > > ============================================================================= > In Emacs 28 and Emacs 29, desktop-load crashes when there are too many > iconified frames in the .emacs.desktop file. > > The two attached .emacs.desktop files show the smallest test case that > I could come up with: > > - .emacs.desktop.works with 3 non-iconified and 4 iconified frames > loads without problem in Emacs 28 > > + desktop-load of this file crashes in Emacs 29.1 > > + a .emacs.desktop with 3 non-iconified and 3 iconified frames works > in Emacs 29.1 > > - .emacs.desktop.crashes with 3 non-iconified and 5 iconified frames > leads to a crash in Emacs 28 (see attached problem report generated > by macOS) > > - I tried .emacs.desktop with up to 17 non-iconified frames in Emacs > 28 without problems > > - Up to Emacs 27, I've never seen a problem with desktop-load crashing > ============================================================================= Can you please try the master branch with the two attached patches applied? The v2 patch should be applied first. The apple crash report is enough to see that the crash is happening somewhere deep inside the AppKit code on frame creation. No idea why, but maybe we'll be lucky and the fix will be the same as the last frame-creation crash fix... -- Alan Third --AFEQo+OQYIoInont Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0001-Simplify-the-EmacsLayer-double-buffering-code-bug.patch" >From 72b13fa67981c40f0e3c63092aa796525fe61344 Mon Sep 17 00:00:00 2001 From: Alan Third Date: Sun, 23 Jul 2023 12:00:30 +0100 Subject: [PATCH v2] Simplify the EmacsLayer double buffering code (bug#63187) --- src/nsfns.m | 30 +++++++++++++++++++++++++++++- src/nsterm.h | 13 ++++++++++++- src/nsterm.m | 50 ++++++++++++++++++++++++++------------------------ 3 files changed, 67 insertions(+), 26 deletions(-) diff --git a/src/nsfns.m b/src/nsfns.m index a79892f73b6..082e06698b2 100644 --- a/src/nsfns.m +++ b/src/nsfns.m @@ -799,6 +799,26 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side. } } +static void +ns_set_inhibit_double_buffering (struct frame *f, + Lisp_Object new_value, + Lisp_Object old_value) +{ +#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400 + if (!EQ (new_value, old_value)) + { + FRAME_DOUBLE_BUFFERED (f) = NILP (new_value); + + /* If the view or layer haven't been created yet this will be a + noop. */ + [(EmacsLayer *)[FRAME_NS_VIEW (f) layer] + setDoubleBuffered:FRAME_DOUBLE_BUFFERED (f)]; + + SET_FRAME_GARBAGED (f); + } +#endif +} + static void ns_set_internal_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval) { @@ -1073,7 +1093,7 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side. gui_set_alpha, 0, /* x_set_sticky */ ns_set_tool_bar_position, - 0, /* x_set_inhibit_double_buffering */ + ns_set_inhibit_double_buffering, ns_set_undecorated, ns_set_parent_frame, 0, /* x_set_skip_taskbar */ @@ -1461,6 +1481,14 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side. gui_default_parameter (f, parms, Qtitle, Qnil, "title", "Title", RES_TYPE_STRING); +#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400 + tem = gui_display_get_arg (dpyinfo, parms, Qinhibit_double_buffering, NULL, NULL, + RES_TYPE_BOOLEAN); + FRAME_DOUBLE_BUFFERED (f) = NILP (tem) || EQ (tem, Qunbound); + store_frame_param (f, Qinhibit_double_buffering, + FRAME_DOUBLE_BUFFERED (f) ? Qnil : Qt); +#endif + parms = get_geometry_from_preferences (dpyinfo, parms); window_prompting = gui_figure_window_size (f, parms, false, true); diff --git a/src/nsterm.h b/src/nsterm.h index b6e5a813a6d..8d6c58290cc 100644 --- a/src/nsterm.h +++ b/src/nsterm.h @@ -746,9 +746,11 @@ #define NSTRACE_UNSILENCE() CGColorSpaceRef colorSpace; IOSurfaceRef currentSurface; CGContextRef context; + bool doubleBuffered; } -- (id) initWithColorSpace: (CGColorSpaceRef)cs; +- (id) initWithColorSpace: (CGColorSpaceRef)cs doubleBuffered: (bool)db; - (void) setColorSpace: (CGColorSpaceRef)cs; +- (void) setDoubleBuffered: (bool)db; - (CGContextRef) getContext; @end #endif @@ -996,6 +998,11 @@ #define KEY_NS_SHOW_PREFS ((1<<28)|(0<<16)|14) /* Non-zero if we are doing an animation, e.g. toggling the tool bar. */ int in_animation; +#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400 + /* Is the frame double buffered? */ + bool double_buffered; +#endif + #ifdef NS_IMPL_GNUSTEP /* Zero if this is the first time a toolbar has been updated on this frame. */ @@ -1030,6 +1037,10 @@ #define FRAME_POINTER_TYPE(f) ((f)->output_data.ns->current_pointer) #define FRAME_FONT(f) ((f)->output_data.ns->font) +#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400 +#define FRAME_DOUBLE_BUFFERED(f) ((f)->output_data.ns->double_buffered) +#endif + #ifdef __OBJC__ #define XNS_SCROLL_BAR(vec) ((id) xmint_pointer (vec)) #else diff --git a/src/nsterm.m b/src/nsterm.m index 78089906752..28502ad1a2a 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -2704,11 +2704,10 @@ Hide the window (X11 semantics) { NSRect srcRect = NSMakeRect (x, from_y, width, height); NSPoint dest = NSMakePoint (x, to_y); - NSRect destRect = NSMakeRect (x, from_y, width, height); EmacsView *view = FRAME_NS_VIEW (f); [view copyRect:srcRect to:dest]; -#ifdef NS_IMPL_COCOA +#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED < 101400 [view setNeedsDisplayInRect:destRect]; #endif } @@ -8607,7 +8606,8 @@ - (instancetype)toggleToolbar: (id)sender - (CALayer *)makeBackingLayer { EmacsLayer *l = [[EmacsLayer alloc] - initWithColorSpace:[[[self window] colorSpace] CGColorSpace]]; + initWithColorSpace:[[[self window] colorSpace] CGColorSpace] + doubleBuffered:FRAME_DOUBLE_BUFFERED (emacsframe)]; [l setDelegate:(id)self]; [l setContentsScale:[[self window] backingScaleFactor]]; @@ -8664,8 +8664,10 @@ - (void)copyRect:(NSRect)srcRect to:(NSPoint)dest NSHeight (srcRect)); #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400 - double scale = [[self window] backingScaleFactor]; CGContextRef context = [(EmacsLayer *)[self layer] getContext]; + CGContextFlush (context); + + double scale = [[self window] backingScaleFactor]; int bpp = CGBitmapContextGetBitsPerPixel (context) / 8; void *pixels = CGBitmapContextGetData (context); int rowSize = CGBitmapContextGetBytesPerRow (context); @@ -10435,22 +10437,20 @@ @implementation EmacsLayer cache. If no free surfaces are found in the cache then a new one is created. */ -#define CACHE_MAX_SIZE 2 - - (id) initWithColorSpace: (CGColorSpaceRef)cs + doubleBuffered: (bool)db { - NSTRACE ("[EmacsLayer initWithColorSpace:]"); + NSTRACE ("[EmacsLayer initWithColorSpace:doubleBuffered:]"); self = [super init]; if (self) { - cache = [[NSMutableArray arrayWithCapacity:CACHE_MAX_SIZE] retain]; [self setColorSpace:cs]; + [self setDoubleBuffered:db]; + cache = [[NSMutableArray arrayWithCapacity:(doubleBuffered ? 2 : 1)] retain]; } else - { - return nil; - } + return nil; return self; } @@ -10467,6 +10467,15 @@ - (void) setColorSpace: (CGColorSpaceRef)cs } +- (void) setDoubleBuffered: (bool)db +{ + if (doubleBuffered != db) + [self releaseSurfaces]; + + doubleBuffered = db; +} + + - (void) dealloc { [self releaseSurfaces]; @@ -10538,7 +10547,7 @@ - (CGContextRef) getContext } } - if (!surface && [cache count] >= CACHE_MAX_SIZE) + if (!surface && [cache count] >= (doubleBuffered ? 2 : 1)) { /* Just grab the first one off the cache. This may result in tearing effects. The alternative is to wait for one @@ -10591,7 +10600,7 @@ - (CGContextRef) getContext return nil; } - CGContextTranslateCTM(context, 0, IOSurfaceGetHeight (currentSurface)); + CGContextTranslateCTM(context, 0, IOSurfaceGetHeight (surface)); CGContextScaleCTM(context, scale, -scale); } @@ -10608,6 +10617,7 @@ - (void) releaseContext if (!context) return; + CGContextFlush (context); CGContextRelease (context); context = NULL; @@ -10621,26 +10631,18 @@ - (void) display { NSTRACE_WHEN (NSTRACE_GROUP_FOCUS, "[EmacsLayer display]"); - if (context) + if (context && context != [[NSGraphicsContext currentContext] CGContext]) { [self releaseContext]; -#if CACHE_MAX_SIZE == 1 - /* This forces the layer to see the surface as updated. */ + /* This forces the layer to see the surface as updated even if + we replace it with itself. */ [self setContents:nil]; -#endif - [self setContents:(id)currentSurface]; /* Put currentSurface back on the end of the cache. */ [cache addObject:(id)currentSurface]; currentSurface = NULL; - - /* Schedule a run of getContext so that if Emacs is idle it will - perform the buffer copy, etc. */ - [self performSelectorOnMainThread:@selector (getContext) - withObject:nil - waitUntilDone:NO]; } } -- 2.40.1 --AFEQo+OQYIoInont Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-Fix-crash-on-child-frame-creation-bug-65817.patch" >From 893f079f51c3bc81d8719c48fe539f72b1025fb8 Mon Sep 17 00:00:00 2001 From: Alan Third Date: Fri, 8 Sep 2023 19:33:06 +0100 Subject: [PATCH] Fix crash on child frame creation (bug#65817) * src/nsterm.m ([EmacsView initFrameFromEmacs:]): Reorder the way the frame and layers are created. ([EmacsView makeBackingLayer]): Change to the newly renamed method below. ([EmacsLayer initWithColorSpace:doubleBuffered:]): ([EmacsLayer initWithDoubleBuffered:]): Rename the method and remove the colorspace argument as it's no longer able to be set on initial creation. * src/nsterm.h: Use new method prototype. --- src/nsterm.h | 2 +- src/nsterm.m | 25 +++++++++++++------------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/nsterm.h b/src/nsterm.h index 8d6c58290cc..cb162039ad8 100644 --- a/src/nsterm.h +++ b/src/nsterm.h @@ -748,7 +748,7 @@ #define NSTRACE_UNSILENCE() CGContextRef context; bool doubleBuffered; } -- (id) initWithColorSpace: (CGColorSpaceRef)cs doubleBuffered: (bool)db; +- (id) initWithDoubleBuffered: (bool)db; - (void) setColorSpace: (CGColorSpaceRef)cs; - (void) setDoubleBuffered: (bool)db; - (CGContextRef) getContext; diff --git a/src/nsterm.m b/src/nsterm.m index 28502ad1a2a..a3603171bb0 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -7921,8 +7921,6 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f maximizing_resize = NO; #endif - [[EmacsWindow alloc] initWithEmacsFrame:f]; - #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400 /* These settings mean AppKit will retain the contents of the frame on resize. Unfortunately it also means the frame will not be @@ -7933,9 +7931,14 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f NSViewLayerContentsRedrawOnSetNeedsDisplay]; [self setLayerContentsPlacement:NSViewLayerContentsPlacementTopLeft]; - /* initWithEmacsFrame can't create the toolbar before the layer is - set, so have another go at creating the toolbar here. */ - [(EmacsWindow*)[self window] createToolbar:f]; + [[EmacsWindow alloc] initWithEmacsFrame:f]; + + /* Now the NSWindow has been created, we can finish up configuring + the layer. */ + [(EmacsLayer *)[self layer] setColorSpace:[[[self window] colorSpace] CGColorSpace]]; + [(EmacsLayer *)[self layer] setContentsScale:[[self window] backingScaleFactor]]; +#else + [[EmacsWindow alloc] initWithEmacsFrame:f]; #endif if (ns_drag_types) @@ -8606,10 +8609,9 @@ - (instancetype)toggleToolbar: (id)sender - (CALayer *)makeBackingLayer { EmacsLayer *l = [[EmacsLayer alloc] - initWithColorSpace:[[[self window] colorSpace] CGColorSpace] - doubleBuffered:FRAME_DOUBLE_BUFFERED (emacsframe)]; + initWithDoubleBuffered:FRAME_DOUBLE_BUFFERED (emacsframe)]; + [l setDelegate:(id)self]; - [l setContentsScale:[[self window] backingScaleFactor]]; return l; } @@ -10437,15 +10439,14 @@ @implementation EmacsLayer cache. If no free surfaces are found in the cache then a new one is created. */ -- (id) initWithColorSpace: (CGColorSpaceRef)cs - doubleBuffered: (bool)db +- (id) initWithDoubleBuffered: (bool)db { - NSTRACE ("[EmacsLayer initWithColorSpace:doubleBuffered:]"); + NSTRACE ("[EmacsLayer initWithDoubleBuffered:]"); self = [super init]; if (self) { - [self setColorSpace:cs]; + [self setColorSpace:nil]; [self setDoubleBuffered:db]; cache = [[NSMutableArray arrayWithCapacity:(doubleBuffered ? 2 : 1)] retain]; } -- 2.40.1 --AFEQo+OQYIoInont--