unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#32812: 27.0.50; macOS Mojave GNU Emacs crash after 5-10 minutes or so
@ 2018-09-23 15:21 Zack Piper
  2018-09-24 10:17 ` Alan Third
  0 siblings, 1 reply; 7+ messages in thread
From: Zack Piper @ 2018-09-23 15:21 UTC (permalink / raw)
  To: 32812


[-- Attachment #1.1: Type: text/plain, Size: 12324 bytes --]

Hi!

(Replacing my post in #31904 with this since they're separate issues(?))

I've applied a slightly modified (i.e. updated) version of Alan's patch 
from bug #31904
(attached) to fix an issue concerning the buffer and mode-line not being 
rendered. The patch works great, everything is now rendered.

Now I notice that after 5-10 minutes (or even before), Emacs crashes 
with the below:

```
objc[82784]: Invalid or prematurely-freed autorelease pool 0x1030032e0.
```

This started occurring since upgrading to macOS Mojave, so possibly 
there's a bug with Mojave or just some incompatibility, not really sure!


(I'm hoping I got it compiled with the appropriate parameters for 
debugging)

The crash isn't intermittent, just takes a variable amount of time to 
trigger. It
can trigger regardless of me using Emacs (i.e. typing) or not.

Also worthy to note this happens with the original patch applied to the 
emacs-26 branch.


Below is a backtrace of the issue:

```
objc[95306]: Invalid or prematurely-freed autorelease pool 0x103e60378.
Process 95306 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = signal 
SIGABRT
     frame #0: 0x00007fff7679b01e 
libsystem_kernel.dylib`__abort_with_payload + 10
libsystem_kernel.dylib`__abort_with_payload:
->  0x7fff7679b01e <+10>: jae    0x7fff7679b028            ; <+20>
     0x7fff7679b020 <+12>: movq   %rax, %rdi
     0x7fff7679b023 <+15>: jmp    0x7fff7677de67            ; 
cerror_nocancel
     0x7fff7679b028 <+20>: retq
Target 0: (Emacs) stopped.
(lldb) bt all
* thread #1, queue = 'com.apple.main-thread', stop reason = signal 
SIGABRT
   * frame #0: 0x00007fff7679b01e 
libsystem_kernel.dylib`__abort_with_payload + 10
     frame #1: 0x00007fff76796561 
libsystem_kernel.dylib`abort_with_payload_wrapper_internal + 82
     frame #2: 0x00007fff7679650f 
libsystem_kernel.dylib`abort_with_reason + 22
     frame #3: 0x00007fff75579674 libobjc.A.dylib`_objc_fatalv(unsigned 
long long, unsigned long long, char const*, __va_list_tag*) + 108
     frame #4: 0x00007fff75579526 libobjc.A.dylib`_objc_fatal(char 
const*, ...) + 135
     frame #5: 0x00007fff7556bd73 libobjc.A.dylib`(anonymous 
namespace)::AutoreleasePoolPage::pop(void*) + 957
     frame #6: 0x00007fff46c2d232 AppKit`-[NSView(NSInternal) 
_recursive:displayRectIgnoringOpacity:inContext:shouldChangeFontReferenceColor:stopAtLayerBackedViews:] 
+ 3454
     frame #7: 0x00007fff46c2c4a2 AppKit`__46-[NSView(NSLayerKitGlue) 
drawLayer:inContext:]_block_invoke + 192
     frame #8: 0x00007fff46c2c201 AppKit`-[NSView(NSLayerKitGlue) 
_drawViewBackingLayer:inContext:drawingHandler:] + 1769
     frame #9: 0x00007fff54540aaf QuartzCore`CABackingStoreUpdate_ + 577
     frame #10: 0x00007fff545a2325 
QuartzCore`___ZN2CA5Layer8display_Ev_block_invoke + 53
     frame #11: 0x00007fff5453fc90 QuartzCore`-[CALayer _display] + 1839
     frame #12: 0x00007fff46c2b75a AppKit`_NSBackingLayerDisplay + 531
     frame #13: 0x00007fff46c0fcc9 AppKit`-[_NSViewBackingLayer display] 
+ 811
     frame #14: 0x00007fff46c0f949 AppKit`_NSBackingLayerDisplayIfNeeded 
+ 40
     frame #15: 0x00007fff46c0f2a4 AppKit`-[NSView displayIfNeeded] + 
130
     frame #16: 0x0000000100026514 Emacs`echo_area_display + 593
     frame #17: 0x00000001000261ca Emacs`message3_nolog + 393
     frame #18: 0x0000000100025fe3 Emacs`message3 + 399
     frame #19: 0x0000000100106a03 Emacs`Fmessage + 67
     frame #20: 0x000000010010f525 Emacs`Ffuncall + 665
     frame #21: 0x0000000100145e02 Emacs`exec_byte_code + 1486
     frame #22: 0x0000000100110040 Emacs`funcall_lambda + 648
     frame #23: 0x000000010010f4c2 Emacs`Ffuncall + 566
     frame #24: 0x000000010010f704 Emacs`funcall_nil + 9
     frame #25: 0x000000010010f6a0 Emacs`run_hook_with_args + 198
     frame #26: 0x000000010010f58b Emacs`Frun_hooks + 60
     frame #27: 0x000000010010f525 Emacs`Ffuncall + 665
     frame #28: 0x0000000100145e02 Emacs`exec_byte_code + 1486
     frame #29: 0x000000010010f4c2 Emacs`Ffuncall + 566
     frame #30: 0x0000000100145e02 Emacs`exec_byte_code + 1486
     frame #31: 0x000000010010f4c2 Emacs`Ffuncall + 566
     frame #32: 0x0000000100145e02 Emacs`exec_byte_code + 1486
     frame #33: 0x000000010010f4c2 Emacs`Ffuncall + 566
     frame #34: 0x0000000100145e02 Emacs`exec_byte_code + 1486
     frame #35: 0x000000010010f4c2 Emacs`Ffuncall + 566
     frame #36: 0x0000000100145e02 Emacs`exec_byte_code + 1486
     frame #37: 0x000000010010f4c2 Emacs`Ffuncall + 566
     frame #38: 0x0000000100145e02 Emacs`exec_byte_code + 1486
     frame #39: 0x000000010010ee62 Emacs`apply_lambda + 369
     frame #40: 0x000000010010c484 Emacs`eval_sub + 845
     frame #41: 0x000000010012d294 Emacs`readevalloop + 1773
     frame #42: 0x000000010012d502 Emacs`Feval_buffer + 368
     frame #43: 0x000000010010fcfe Emacs`funcall_subr + 367
     frame #44: 0x000000010010f525 Emacs`Ffuncall + 665
     frame #45: 0x0000000100145e02 Emacs`exec_byte_code + 1486
     frame #46: 0x0000000100110040 Emacs`funcall_lambda + 648
     frame #47: 0x000000010010f4c2 Emacs`Ffuncall + 566
     frame #48: 0x000000010010f9d1 Emacs`call4 + 58
     frame #49: 0x000000010012b845 Emacs`Fload + 1373
     frame #50: 0x000000010010fcfe Emacs`funcall_subr + 367
     frame #51: 0x000000010010f525 Emacs`Ffuncall + 665
     frame #52: 0x0000000100145e02 Emacs`exec_byte_code + 1486
     frame #53: 0x000000010010f4c2 Emacs`Ffuncall + 566
     frame #54: 0x0000000100145e02 Emacs`exec_byte_code + 1486
     frame #55: 0x000000010010f4c2 Emacs`Ffuncall + 566
     frame #56: 0x0000000100145e02 Emacs`exec_byte_code + 1486
     frame #57: 0x000000010010ee62 Emacs`apply_lambda + 369
     frame #58: 0x000000010010c484 Emacs`eval_sub + 845
     frame #59: 0x000000010010ec69 Emacs`Feval + 96
     frame #60: 0x000000010010dffd Emacs`internal_condition_case + 87
     frame #61: 0x00000001000b17be Emacs`top_level_1 + 45
     frame #62: 0x000000010010db85 Emacs`internal_catch + 74
     frame #63: 0x00000001000a4a90 Emacs`command_loop + 141
     frame #64: 0x00000001000a49c1 Emacs`recursive_edit_1 + 115
     frame #65: 0x00000001000a4bdd Emacs`Frecursive_edit + 226
     frame #66: 0x00000001000a3aa9 Emacs`main + 5211
     frame #67: 0x00007fff76645085 libdyld.dylib`start + 1
     frame #68: 0x00007fff76645085 libdyld.dylib`start + 1
   thread #2
     frame #0: 0x00007fff7677f5be 
libsystem_kernel.dylib`__workq_kernreturn + 10
     frame #1: 0x6874656d00000000
     frame #2: 0x00007fff76836415 libsystem_pthread.dylib`start_wqthread 
+ 13
   thread #3
     frame #0: 0x00007fff7677f5be 
libsystem_kernel.dylib`__workq_kernreturn + 10
     frame #1: 0x00007fff76836641 
libsystem_pthread.dylib`_pthread_wqthread + 446
     frame #2: 0x00007fff76836415 libsystem_pthread.dylib`start_wqthread 
+ 13
   thread #4, name = 'gmain'
     frame #0: 0x00007fff76784e82 libsystem_kernel.dylib`__select + 10
     frame #1: 0x0000000100a083da libglib-2.0.0.dylib`g_poll + 405
     frame #2: 0x00000001009fc123 
libglib-2.0.0.dylib`g_main_context_iterate + 340
     frame #3: 0x00000001009fc1d1 
libglib-2.0.0.dylib`g_main_context_iteration + 55
     frame #4: 0x00000001009fd2b0 libglib-2.0.0.dylib`glib_worker_main + 
30
     frame #5: 0x0000000100a1dcb7 libglib-2.0.0.dylib`g_thread_proxy + 
90
     frame #6: 0x00007fff7683733d libsystem_pthread.dylib`_pthread_body 
+ 126
     frame #7: 0x00007fff7683a2a7 libsystem_pthread.dylib`_pthread_start 
+ 70
     frame #8: 0x00007fff76836425 libsystem_pthread.dylib`thread_start + 
13
   thread #5
     frame #0: 0x00007fff7677f5be 
libsystem_kernel.dylib`__workq_kernreturn + 10
     frame #1: 0x00007fff76836721 
libsystem_pthread.dylib`_pthread_wqthread + 670
     frame #2: 0x00007fff76836415 libsystem_pthread.dylib`start_wqthread 
+ 13
   thread #7
     frame #0: 0x00007fff76784e82 libsystem_kernel.dylib`__select + 10
     frame #1: 0x000000010017c408 Emacs`-[EmacsApp fd_handler:] + 214
     frame #2: 0x00007fff4b902234 Foundation`__NSThread__start__ + 1218
     frame #3: 0x00007fff7683733d libsystem_pthread.dylib`_pthread_body 
+ 126
     frame #4: 0x00007fff7683a2a7 libsystem_pthread.dylib`_pthread_start 
+ 70
     frame #5: 0x00007fff76836425 libsystem_pthread.dylib`thread_start + 
13
   thread #9, name = 'com.apple.NSEventThread'
     frame #0: 0x00007fff7677dc2a libsystem_kernel.dylib`mach_msg_trap + 
10
     frame #1: 0x00007fff7677e174 libsystem_kernel.dylib`mach_msg + 60
     frame #2: 0x00007fff495ad05e 
CoreFoundation`__CFRunLoopServiceMachPort + 337
     frame #3: 0x00007fff495ac5ad CoreFoundation`__CFRunLoopRun + 1654
     frame #4: 0x00007fff495abce4 CoreFoundation`CFRunLoopRunSpecific + 
463
     frame #5: 0x00007fff46b0b581 AppKit`_NSEventThread + 160
     frame #6: 0x00007fff7683733d libsystem_pthread.dylib`_pthread_body 
+ 126
     frame #7: 0x00007fff7683a2a7 libsystem_pthread.dylib`_pthread_start 
+ 70
     frame #8: 0x00007fff76836425 libsystem_pthread.dylib`thread_start + 
13
(lldb)
```






In GNU Emacs 27.0.50 (build 1, x86_64-apple-darwin18.0.0, NS 
appkit-1671.00 Version 10.14 (Build 18A389))
  of 2018-09-19 built on zoral
Repository revision: 75d9a55fae1c484aa6d213064931bfe3b65cf5dd
Windowing system distributor 'Apple', version 10.3.1671
System Description:  Mac OS X 10.14

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
  'configure --disable-dependency-tracking --disable-silent-rules
  --enable-locallisppath=/usr/local/share/emacs/site-lisp
  --enable-check-lisp-object-type
  --infodir=/usr/local/Cellar/emacs-plus/HEAD-75d9a55/share/info/emacs
  --prefix=/usr/local/Cellar/emacs-plus/HEAD-75d9a55 --with-xml2
  --without-dbus --with-gnutls --with-imagemagick --with-modules
  --with-rsvg --with-ns --disable-ns-self-contained 'CFLAGS=-O0 -g3''

Configured features:
RSVG IMAGEMAGICK GLIB NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS
NS MODULES THREADS LCMS2 GMP

Important settings:
   value of $LANG: en_GB.UTF-8
   locale-coding-system: utf-8-unix

Major mode: Fundamental

Minor modes in effect:
   tooltip-mode: t
   global-eldoc-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   blink-cursor-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   buffer-read-only: t
   line-number-mode: t
   transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq byte-opt gv
bytecomp byte-compile cconv dired dired-loaddefs format-spec rfc822 mml
easymenu mml-sec password-cache epa derived epg epg-config gnus-util
rmail rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader cl-loaddefs cl-lib sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils elec-pair time-date
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/ns-win ns-win ucs-normalize mule-util term/common-win
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow isearch timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core term/tty-colors frame cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads kqueue cocoa ns
lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 204840 9831)
  (symbols 48 20275 1)
  (strings 32 29150 2191)
  (string-bytes 1 776518)
  (vectors 16 35468)
  (vector-slots 8 735114 19448)
  (floats 8 46 70)
  (intervals 56 229 0)
  (buffers 992 12))

[-- Attachment #1.2: Type: text/html, Size: 13509 bytes --]

[-- Attachment #2: 0001-nsterm.m.patch --]
[-- Type: text/plain, Size: 34371 bytes --]

From dea03ff92e307456d103f716652ae76987da02f9 Mon Sep 17 00:00:00 2001
From: Zack Piper <zack@apertron.com>
Date: Tue, 18 Sep 2018 21:42:55 +0100
Subject: [PATCH] nsterm.m

---
 src/nsterm.m | 735 +++++++++++++++++++++++++--------------------------
 1 file changed, 364 insertions(+), 371 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index 961271f2d0..a2117eb027 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -278,8 +278,6 @@ - (NSColor *)colorUsingDefaultColorSpace
 long context_menu_value = 0;
 
 /* display update */
-static struct frame *ns_updating_frame;
-static NSView *focus_view = NULL;
 static int ns_window_num = 0;
 #ifdef NS_IMPL_GNUSTEP
 static NSRect uRect;            // TODO: This is dead, remove it?
@@ -1109,30 +1107,29 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
   }
 #endif
 
-  ns_updating_frame = f;
-  [view lockFocus];
-
   /* drawRect may have been called for say the minibuffer, and then clip path
      is for the minibuffer.  But the display engine may draw more because
      we have set the frame as garbaged.  So reset clip path to the whole
      view.  */
+  /* FIXME: I don't think we need to do this.  */
 #ifdef NS_IMPL_COCOA
-  {
-    NSBezierPath *bp;
-    NSRect r = [view frame];
-    NSRect cr = [[view window] frame];
-    /* If a large frame size is set, r may be larger than the window frame
-       before constrained.  In that case don't change the clip path, as we
-       will clear in to the tool bar and title bar.  */
-    if (r.size.height
-        + FRAME_NS_TITLEBAR_HEIGHT (f)
-        + FRAME_TOOLBAR_HEIGHT (f) <= cr.size.height)
-      {
-        bp = [[NSBezierPath bezierPathWithRect: r] retain];
-        [bp setClip];
-        [bp release];
-      }
-  }
+  if ([NSView focusView] == FRAME_NS_VIEW (f))
+    {
+      NSBezierPath *bp;
+      NSRect r = [view frame];
+      NSRect cr = [[view window] frame];
+      /* If a large frame size is set, r may be larger than the window frame
+         before constrained.  In that case don't change the clip path, as we
+         will clear in to the tool bar and title bar.  */
+      if (r.size.height
+          + FRAME_NS_TITLEBAR_HEIGHT (f)
+          + FRAME_TOOLBAR_HEIGHT (f) <= cr.size.height)
+        {
+          bp = [[NSBezierPath bezierPathWithRect: r] retain];
+          [bp setClip];
+          [bp release];
+        }
+    }
 #endif
 
 #ifdef NS_IMPL_GNUSTEP
@@ -1218,23 +1215,14 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
    external (RIF) call; for whole frame, called after update_window_end
    -------------------------------------------------------------------------- */
 {
-  EmacsView *view = FRAME_NS_VIEW (f);
-
   NSTRACE_WHEN (NSTRACE_GROUP_UPDATES, "ns_update_end");
 
 /*   if (f == MOUSE_HL_INFO (f)->mouse_face_mouse_frame) */
   MOUSE_HL_INFO (f)->mouse_face_defer = 0;
-
-  block_input ();
-
-  [view unlockFocus];
-  [[view window] flushWindow];
-
-  unblock_input ();
-  ns_updating_frame = NULL;
 }
 
-static void
+
+static BOOL
 ns_focus (struct frame *f, NSRect *r, int n)
 /* --------------------------------------------------------------------------
    Internal: Focus on given frame.  During small local updates this is used to
@@ -1246,48 +1234,38 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
    -------------------------------------------------------------------------- */
 {
   NSTRACE_WHEN (NSTRACE_GROUP_FOCUS, "ns_focus");
-  if (r != NULL)
+  if (r)
     {
       NSTRACE_RECT ("r", *r);
-    }
 
-  if (f != ns_updating_frame)
-    {
-      NSView *view = FRAME_NS_VIEW (f);
-      if (view != focus_view)
+      if ([NSView focusView] == FRAME_NS_VIEW (f))
         {
-          if (focus_view != NULL)
-            {
-              [focus_view unlockFocus];
-              [[focus_view window] flushWindow];
-/* debug_lock--; */
-            }
+          [[NSGraphicsContext currentContext] saveGraphicsState];
+          if (n == 2)
+            NSRectClipList (r, 2);
+          else
+            NSRectClip (*r);
+          gsaved = YES;
 
-          if (view)
-            [view lockFocus];
-          focus_view = view;
-/* if (view) debug_lock++; */
+          return YES;
         }
-    }
-
-  /* clipping */
-  if (r)
-    {
-      [[NSGraphicsContext currentContext] saveGraphicsState];
-      if (n == 2)
-        NSRectClipList (r, 2);
       else
-        NSRectClip (*r);
-      gsaved = YES;
+        {
+          NSView *view = FRAME_NS_VIEW (f);
+          int i;
+          for (i = 0 ; i < n ; i++)
+            [view setNeedsDisplayInRect:r[i]];
+        }
     }
+
+  return NO;
 }
 
 
 static void
 ns_unfocus (struct frame *f)
-/* --------------------------------------------------------------------------
-     Internal: Remove focus on given frame
-   -------------------------------------------------------------------------- */
+/* Internal: Restore the previous graphics state, unsetting any
+   clipping areas.  */
 {
   NSTRACE_WHEN (NSTRACE_GROUP_FOCUS, "ns_unfocus");
 
@@ -1297,20 +1275,10 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
       gsaved = NO;
     }
 
-  if (f != ns_updating_frame)
-    {
-      if (focus_view != NULL)
-        {
-          [focus_view unlockFocus];
-          [[focus_view window] flushWindow];
-          focus_view = NULL;
-/* debug_lock--; */
-        }
-    }
 }
 
 
-static void
+static BOOL
 ns_clip_to_row (struct window *w, struct glyph_row *row,
 		enum glyph_row_area area, BOOL gc)
 /* --------------------------------------------------------------------------
@@ -1329,7 +1297,17 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
   clip_rect.size.width = window_width;
   clip_rect.size.height = row->visible_height;
 
-  ns_focus (f, &clip_rect, 1);
+  return ns_focus (f, &clip_rect, 1);
+}
+
+
+static void
+ns_flush_display (struct frame *f)
+/* Force the frame to redisplay.  If areas have previously been marked
+   dirty by setNeedsDisplayInRect (in ns_focus), then this will call
+   draw_rect: which will "expose" those areas.  */
+{
+  [FRAME_NS_VIEW (f) displayIfNeeded];
 }
 
 
@@ -2826,14 +2804,16 @@ so some key presses (TAB) are swallowed by the system.  */
   r = [view bounds];
 
   block_input ();
-  ns_focus (f, &r, 1);
-  [ns_lookup_indexed_color (NS_FACE_BACKGROUND
-			    (FACE_FROM_ID (f, DEFAULT_FACE_ID)), f) set];
-  NSRectFill (r);
-  ns_unfocus (f);
-
-  /* as of 2006/11 or so this is now needed */
-  ns_redraw_scroll_bars (f);
+  if (ns_focus (f, &r, 1))
+    {
+      [ns_lookup_indexed_color (NS_FACE_BACKGROUND
+                                (FACE_FROM_ID (f, DEFAULT_FACE_ID)), f) set];
+      NSRectFill (r);
+      ns_unfocus (f);
+
+      /* as of 2006/11 or so this is now needed */
+      ns_redraw_scroll_bars (f);
+    }
   unblock_input ();
 }
 
@@ -2854,13 +2834,14 @@ so some key presses (TAB) are swallowed by the system.  */
   NSTRACE_WHEN (NSTRACE_GROUP_UPDATES, "ns_clear_frame_area");
 
   r = NSIntersectionRect (r, [view frame]);
-  ns_focus (f, &r, 1);
-  [ns_lookup_indexed_color (NS_FACE_BACKGROUND (face), f) set];
+  if (ns_focus (f, &r, 1))
+    {
+      [ns_lookup_indexed_color (NS_FACE_BACKGROUND (face), f) set];
 
-  NSRectFill (r);
+      NSRectFill (r);
 
-  ns_unfocus (f);
-  return;
+      ns_unfocus (f);
+    }
 }
 
 static void
@@ -2872,11 +2853,11 @@ so some key presses (TAB) are swallowed by the system.  */
     {
       hide_bell();              // Ensure the bell image isn't scrolled.
 
-      ns_focus (f, &dest, 1);
+      /* FIXME: scrollRect:by: is deprecated in macOS 10.14.  There is
+         no obvious replacement so we may have to come up with our own.  */
       [FRAME_NS_VIEW (f) scrollRect: src
                                  by: NSMakeSize (dest.origin.x - src.origin.x,
                                                  dest.origin.y - src.origin.y)];
-      ns_unfocus (f);
     }
 }
 
@@ -3087,88 +3068,87 @@ so some key presses (TAB) are swallowed by the system.  */
     }
 
   /* Must clip because of partially visible lines.  */
-  ns_clip_to_row (w, row, ANY_AREA, YES);
-
-  if (!p->overlay_p)
+  if (ns_clip_to_row (w, row, ANY_AREA, YES))
     {
-      int bx = p->bx, by = p->by, nx = p->nx, ny = p->ny;
-
-      if (bx >= 0 && nx > 0)
+      if (!p->overlay_p)
         {
-          NSRect r = NSMakeRect (bx, by, nx, ny);
-          NSRectClip (r);
-          [ns_lookup_indexed_color (face->background, f) set];
-          NSRectFill (r);
-        }
-    }
-
-  if (p->which)
-    {
-      NSRect r = NSMakeRect (p->x, p->y, p->wd, p->h);
-      EmacsImage *img = bimgs[p->which - 1];
+          int bx = p->bx, by = p->by, nx = p->nx, ny = p->ny;
 
-      if (!img)
-        {
-          // Note: For "periodic" images, allocate one EmacsImage for
-          // the base image, and use it for all dh:s.
-          unsigned short *bits = p->bits;
-          int full_height = p->h + p->dh;
-          int i;
-          unsigned char *cbits = xmalloc (full_height);
-
-          for (i = 0; i < full_height; i++)
-            cbits[i] = bits[i];
-          img = [[EmacsImage alloc] initFromXBM: cbits width: 8
-                                         height: full_height
-                                             fg: 0 bg: 0];
-          bimgs[p->which - 1] = img;
-          xfree (cbits);
+          if (bx >= 0 && nx > 0)
+            {
+              NSRect r = NSMakeRect (bx, by, nx, ny);
+              NSRectClip (r);
+              [ns_lookup_indexed_color (face->background, f) set];
+              NSRectFill (r);
+            }
         }
 
-      NSTRACE_RECT ("r", r);
+      if (p->which)
+        {
+          NSRect r = NSMakeRect (p->x, p->y, p->wd, p->h);
+          EmacsImage *img = bimgs[p->which - 1];
 
-      NSRectClip (r);
-      /* Since we composite the bitmap instead of just blitting it, we need
-         to erase the whole background.  */
-      [ns_lookup_indexed_color(face->background, f) set];
-      NSRectFill (r);
+          if (!img)
+            {
+              // Note: For "periodic" images, allocate one EmacsImage for
+              // the base image, and use it for all dh:s.
+              unsigned short *bits = p->bits;
+              int full_height = p->h + p->dh;
+              int i;
+              unsigned char *cbits = xmalloc (full_height);
+
+              for (i = 0; i < full_height; i++)
+                cbits[i] = bits[i];
+              img = [[EmacsImage alloc] initFromXBM: cbits width: 8
+                                             height: full_height
+                                                 fg: 0 bg: 0];
+              bimgs[p->which - 1] = img;
+              xfree (cbits);
+            }
 
-      {
-        NSColor *bm_color;
-        if (!p->cursor_p)
-          bm_color = ns_lookup_indexed_color(face->foreground, f);
-        else if (p->overlay_p)
-          bm_color = ns_lookup_indexed_color(face->background, f);
-        else
-          bm_color = f->output_data.ns->cursor_color;
-        [img setXBMColor: bm_color];
-      }
+          NSTRACE_RECT ("r", r);
 
-#ifdef NS_IMPL_COCOA
-      // Note: For periodic images, the full image height is "h + hd".
-      // By using the height h, a suitable part of the image is used.
-      NSRect fromRect = NSMakeRect(0, 0, p->wd, p->h);
+          NSRectClip (r);
+          /* Since we composite the bitmap instead of just blitting it, we need
+             to erase the whole background.  */
+          [ns_lookup_indexed_color(face->background, f) set];
+          NSRectFill (r);
 
-      NSTRACE_RECT ("fromRect", fromRect);
+          {
+            NSColor *bm_color;
+            if (!p->cursor_p)
+              bm_color = ns_lookup_indexed_color(face->foreground, f);
+            else if (p->overlay_p)
+              bm_color = ns_lookup_indexed_color(face->background, f);
+            else
+              bm_color = f->output_data.ns->cursor_color;
+            [img setXBMColor: bm_color];
+          }
 
-      [img drawInRect: r
-              fromRect: fromRect
-             operation: NSCompositingOperationSourceOver
-              fraction: 1.0
-           respectFlipped: YES
-                hints: nil];
+#ifdef NS_IMPL_COCOA
+          // Note: For periodic images, the full image height is "h + hd".
+          // By using the height h, a suitable part of the image is used.
+          NSRect fromRect = NSMakeRect(0, 0, p->wd, p->h);
+
+          NSTRACE_RECT ("fromRect", fromRect);
+
+          [img drawInRect: r
+                 fromRect: fromRect
+                operation: NSCompositingOperationSourceOver
+                 fraction: 1.0
+               respectFlipped: YES
+                    hints: nil];
 #else
-      {
-        NSPoint pt = r.origin;
-        pt.y += p->h;
-        [img compositeToPoint: pt operation: NSCompositingOperationSourceOver];
-      }
+          {
+            NSPoint pt = r.origin;
+            pt.y += p->h;
+            [img compositeToPoint: pt operation: NSCompositingOperationSourceOver];
+          }
 #endif
+        }
+      ns_unfocus (f);
     }
-  ns_unfocus (f);
 }
-
-
 static void
 ns_draw_window_cursor (struct window *w, struct glyph_row *glyph_row,
 		       int x, int y, enum text_cursor_kinds cursor_type,
@@ -3248,66 +3228,65 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
   r.size.width = w->phys_cursor_width;
 
   /* Prevent the cursor from being drawn outside the text area.  */
-  ns_clip_to_row (w, glyph_row, TEXT_AREA, NO); /* do ns_focus(f, &r, 1); if remove */
-
-
-  face = FACE_FROM_ID_OR_NULL (f, phys_cursor_glyph->face_id);
-  if (face && NS_FACE_BACKGROUND (face)
-      == ns_index_color (FRAME_CURSOR_COLOR (f), f))
+  if (ns_clip_to_row (w, glyph_row, TEXT_AREA, NO))
     {
-      [ns_lookup_indexed_color (NS_FACE_FOREGROUND (face), f) set];
-      hollow_color = FRAME_CURSOR_COLOR (f);
-    }
-  else
-    [FRAME_CURSOR_COLOR (f) set];
+      face = FACE_FROM_ID_OR_NULL (f, phys_cursor_glyph->face_id);
+      if (face && NS_FACE_BACKGROUND (face)
+          == ns_index_color (FRAME_CURSOR_COLOR (f), f))
+        {
+          [ns_lookup_indexed_color (NS_FACE_FOREGROUND (face), f) set];
+          hollow_color = FRAME_CURSOR_COLOR (f);
+        }
+      else
+        [FRAME_CURSOR_COLOR (f) set];
 
 #ifdef NS_IMPL_COCOA
-  /* TODO: This makes drawing of cursor plus that of phys_cursor_glyph
-           atomic.  Cleaner ways of doing this should be investigated.
-           One way would be to set a global variable DRAWING_CURSOR
-           when making the call to draw_phys..(), don't focus in that
-           case, then move the ns_unfocus() here after that call.  */
-  NSDisableScreenUpdates ();
+      /* TODO: This makes drawing of cursor plus that of phys_cursor_glyph
+         atomic.  Cleaner ways of doing this should be investigated.
+         One way would be to set a global variable DRAWING_CURSOR
+         when making the call to draw_phys..(), don't focus in that
+         case, then move the ns_unfocus() here after that call.  */
+      NSDisableScreenUpdates ();
 #endif
 
-  switch (cursor_type)
-    {
-    case DEFAULT_CURSOR:
-    case NO_CURSOR:
-      break;
-    case FILLED_BOX_CURSOR:
-      NSRectFill (r);
-      break;
-    case HOLLOW_BOX_CURSOR:
-      NSRectFill (r);
-      [hollow_color set];
-      NSRectFill (NSInsetRect (r, 1, 1));
-      [FRAME_CURSOR_COLOR (f) set];
-      break;
-    case HBAR_CURSOR:
-      NSRectFill (r);
-      break;
-    case BAR_CURSOR:
-      s = r;
-      /* If the character under cursor is R2L, draw the bar cursor
-         on the right of its glyph, rather than on the left.  */
-      cursor_glyph = get_phys_cursor_glyph (w);
-      if ((cursor_glyph->resolved_level & 1) != 0)
-        s.origin.x += cursor_glyph->pixel_width - s.size.width;
-
-      NSRectFill (s);
-      break;
-    }
-  ns_unfocus (f);
+      switch (cursor_type)
+        {
+        case DEFAULT_CURSOR:
+        case NO_CURSOR:
+          break;
+        case FILLED_BOX_CURSOR:
+          NSRectFill (r);
+          break;
+        case HOLLOW_BOX_CURSOR:
+          NSRectFill (r);
+          [hollow_color set];
+          NSRectFill (NSInsetRect (r, 1, 1));
+          [FRAME_CURSOR_COLOR (f) set];
+          break;
+        case HBAR_CURSOR:
+          NSRectFill (r);
+          break;
+        case BAR_CURSOR:
+          s = r;
+          /* If the character under cursor is R2L, draw the bar cursor
+             on the right of its glyph, rather than on the left.  */
+          cursor_glyph = get_phys_cursor_glyph (w);
+          if ((cursor_glyph->resolved_level & 1) != 0)
+            s.origin.x += cursor_glyph->pixel_width - s.size.width;
+
+          NSRectFill (s);
+          break;
+        }
+      ns_unfocus (f);
 
-  /* draw the character under the cursor */
-  if (cursor_type != NO_CURSOR)
-    draw_phys_cursor_glyph (w, glyph_row, DRAW_CURSOR);
+      /* draw the character under the cursor */
+      if (cursor_type != NO_CURSOR)
+        draw_phys_cursor_glyph (w, glyph_row, DRAW_CURSOR);
 
 #ifdef NS_IMPL_COCOA
-  NSEnableScreenUpdates ();
+      NSEnableScreenUpdates ();
 #endif
-
+    }
 }
 
 
@@ -3325,12 +3304,14 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
 
   face = FACE_FROM_ID_OR_NULL (f, VERTICAL_BORDER_FACE_ID);
 
-  ns_focus (f, &r, 1);
-  if (face)
-    [ns_lookup_indexed_color(face->foreground, f) set];
+  if (ns_focus (f, &r, 1))
+    {
+      if (face)
+        [ns_lookup_indexed_color(face->foreground, f) set];
 
-  NSRectFill(r);
-  ns_unfocus (f);
+      NSRectFill(r);
+      ns_unfocus (f);
+    }
 }
 
 
@@ -3357,39 +3338,40 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
 
   NSTRACE ("ns_draw_window_divider");
 
-  ns_focus (f, &divider, 1);
-
-  if ((y1 - y0 > x1 - x0) && (x1 - x0 >= 3))
-    /* A vertical divider, at least three pixels wide: Draw first and
-       last pixels differently.  */
+  if (ns_focus (f, &divider, 1))
     {
-      [ns_lookup_indexed_color(color_first, f) set];
-      NSRectFill(NSMakeRect (x0, y0, 1, y1 - y0));
-      [ns_lookup_indexed_color(color, f) set];
-      NSRectFill(NSMakeRect (x0 + 1, y0, x1 - x0 - 2, y1 - y0));
-      [ns_lookup_indexed_color(color_last, f) set];
-      NSRectFill(NSMakeRect (x1 - 1, y0, 1, y1 - y0));
-    }
-  else if ((x1 - x0 > y1 - y0) && (y1 - y0 >= 3))
-    /* A horizontal divider, at least three pixels high: Draw first and
-       last pixels differently.  */
-    {
-      [ns_lookup_indexed_color(color_first, f) set];
-      NSRectFill(NSMakeRect (x0, y0, x1 - x0, 1));
-      [ns_lookup_indexed_color(color, f) set];
-      NSRectFill(NSMakeRect (x0, y0 + 1, x1 - x0, y1 - y0 - 2));
-      [ns_lookup_indexed_color(color_last, f) set];
-      NSRectFill(NSMakeRect (x0, y1 - 1, x1 - x0, 1));
-    }
-  else
-    {
-      /* In any other case do not draw the first and last pixels
-         differently.  */
-      [ns_lookup_indexed_color(color, f) set];
-      NSRectFill(divider);
-    }
+      if ((y1 - y0 > x1 - x0) && (x1 - x0 >= 3))
+        /* A vertical divider, at least three pixels wide: Draw first and
+           last pixels differently.  */
+        {
+          [ns_lookup_indexed_color(color_first, f) set];
+          NSRectFill(NSMakeRect (x0, y0, 1, y1 - y0));
+          [ns_lookup_indexed_color(color, f) set];
+          NSRectFill(NSMakeRect (x0 + 1, y0, x1 - x0 - 2, y1 - y0));
+          [ns_lookup_indexed_color(color_last, f) set];
+          NSRectFill(NSMakeRect (x1 - 1, y0, 1, y1 - y0));
+        }
+      else if ((x1 - x0 > y1 - y0) && (y1 - y0 >= 3))
+        /* A horizontal divider, at least three pixels high: Draw first and
+           last pixels differently.  */
+        {
+          [ns_lookup_indexed_color(color_first, f) set];
+          NSRectFill(NSMakeRect (x0, y0, x1 - x0, 1));
+          [ns_lookup_indexed_color(color, f) set];
+          NSRectFill(NSMakeRect (x0, y0 + 1, x1 - x0, y1 - y0 - 2));
+          [ns_lookup_indexed_color(color_last, f) set];
+          NSRectFill(NSMakeRect (x0, y1 - 1, x1 - x0, 1));
+        }
+      else
+        {
+          /* In any other case do not draw the first and last pixels
+             differently.  */
+          [ns_lookup_indexed_color(color, f) set];
+          NSRectFill(divider);
+        }
 
-  ns_unfocus (f);
+      ns_unfocus (f);
+    }
 }
 
 static void
@@ -3988,83 +3970,84 @@ Function modeled after x_draw_glyph_string_box ().
       n = ns_get_glyph_string_clip_rect (s, r);
       *r = NSMakeRect (s->x, s->y, s->background_width, s->height);
 
-      ns_focus (s->f, r, n);
-
-      if (s->hl == DRAW_MOUSE_FACE)
-       {
-         face = FACE_FROM_ID_OR_NULL (s->f,
-				      MOUSE_HL_INFO (s->f)->mouse_face_face_id);
-         if (!face)
-           face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
-       }
-      else
-       face = FACE_FROM_ID (s->f, s->first_glyph->face_id);
-
-      bgCol = ns_lookup_indexed_color (NS_FACE_BACKGROUND (face), s->f);
-      fgCol = ns_lookup_indexed_color (NS_FACE_FOREGROUND (face), s->f);
-
-      for (i = 0; i < n; ++i)
+      if (ns_focus (s->f, r, n))
         {
-          if (!s->row->full_width_p)
+          if (s->hl == DRAW_MOUSE_FACE)
             {
-	      int overrun, leftoverrun;
-
-              /* truncate to avoid overwriting fringe and/or scrollbar */
-	      overrun = max (0, (s->x + s->background_width)
-			     - (WINDOW_BOX_RIGHT_EDGE_X (s->w)
-				- WINDOW_RIGHT_FRINGE_WIDTH (s->w)));
-              r[i].size.width -= overrun;
-
-	      /* truncate to avoid overwriting to left of the window box */
-	      leftoverrun = (WINDOW_BOX_LEFT_EDGE_X (s->w)
-			     + WINDOW_LEFT_FRINGE_WIDTH (s->w)) - s->x;
-
-	      if (leftoverrun > 0)
-		{
-		  r[i].origin.x += leftoverrun;
-		  r[i].size.width -= leftoverrun;
-		}
-
-              /* XXX: Try to work between problem where a stretch glyph on
-                 a partially-visible bottom row will clear part of the
-                 modeline, and another where list-buffers headers and similar
-                 rows erroneously have visible_height set to 0.  Not sure
-                 where this is coming from as other terms seem not to show.  */
-              r[i].size.height = min (s->height, s->row->visible_height);
+              face = FACE_FROM_ID_OR_NULL (s->f,
+                                           MOUSE_HL_INFO (s->f)->mouse_face_face_id);
+              if (!face)
+                face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
             }
+          else
+            face = FACE_FROM_ID (s->f, s->first_glyph->face_id);
 
-          [bgCol set];
+          bgCol = ns_lookup_indexed_color (NS_FACE_BACKGROUND (face), s->f);
+          fgCol = ns_lookup_indexed_color (NS_FACE_FOREGROUND (face), s->f);
 
-          /* NOTE: under NS this is NOT used to draw cursors, but we must avoid
-             overwriting cursor (usually when cursor on a tab).  */
-          if (s->hl == DRAW_CURSOR)
+          for (i = 0; i < n; ++i)
             {
-              CGFloat x, width;
+              if (!s->row->full_width_p)
+                {
+                  int overrun, leftoverrun;
+
+                  /* truncate to avoid overwriting fringe and/or scrollbar */
+                  overrun = max (0, (s->x + s->background_width)
+                                 - (WINDOW_BOX_RIGHT_EDGE_X (s->w)
+                                    - WINDOW_RIGHT_FRINGE_WIDTH (s->w)));
+                  r[i].size.width -= overrun;
+
+                  /* truncate to avoid overwriting to left of the window box */
+                  leftoverrun = (WINDOW_BOX_LEFT_EDGE_X (s->w)
+                                 + WINDOW_LEFT_FRINGE_WIDTH (s->w)) - s->x;
+
+                    if (leftoverrun > 0)
+                      {
+                        r[i].origin.x += leftoverrun;
+                        r[i].size.width -= leftoverrun;
+                      }
+
+                    /* XXX: Try to work between problem where a stretch glyph on
+                       a partially-visible bottom row will clear part of the
+                       modeline, and another where list-buffers headers and similar
+                       rows erroneously have visible_height set to 0.  Not sure
+                       where this is coming from as other terms seem not to show.  */
+                    r[i].size.height = min (s->height, s->row->visible_height);
+                }
+
+              [bgCol set];
+
+              /* NOTE: under NS this is NOT used to draw cursors, but we must avoid
+                 overwriting cursor (usually when cursor on a tab).  */
+              if (s->hl == DRAW_CURSOR)
+                {
+                  CGFloat x, width;
 
-              x = r[i].origin.x;
-              width = s->w->phys_cursor_width;
-              r[i].size.width -= width;
-              r[i].origin.x += width;
+                  x = r[i].origin.x;
+                  width = s->w->phys_cursor_width;
+                  r[i].size.width -= width;
+                  r[i].origin.x += width;
 
-              NSRectFill (r[i]);
+                  NSRectFill (r[i]);
 
-              /* Draw overlining, etc. on the cursor.  */
-              if (s->w->phys_cursor_type == FILLED_BOX_CURSOR)
-                ns_draw_text_decoration (s, face, bgCol, width, x);
+                  /* Draw overlining, etc. on the cursor.  */
+                  if (s->w->phys_cursor_type == FILLED_BOX_CURSOR)
+                    ns_draw_text_decoration (s, face, bgCol, width, x);
+                  else
+                    ns_draw_text_decoration (s, face, fgCol, width, x);
+                }
               else
-                ns_draw_text_decoration (s, face, fgCol, width, x);
-            }
-          else
-            {
-              NSRectFill (r[i]);
-            }
+                {
+                  NSRectFill (r[i]);
+                }
 
-          /* Draw overlining, etc. on the stretch glyph (or the part
-             of the stretch glyph after the cursor).  */
-          ns_draw_text_decoration (s, face, fgCol, r[i].size.width,
-                                   r[i].origin.x);
+              /* Draw overlining, etc. on the stretch glyph (or the part
+                 of the stretch glyph after the cursor).  */
+              ns_draw_text_decoration (s, face, fgCol, r[i].size.width,
+                                       r[i].origin.x);
+            }
+          ns_unfocus (s->f);
         }
-      ns_unfocus (s->f);
       s->background_filled_p = 1;
     }
 }
@@ -4214,9 +4197,11 @@ overwriting cursor (usually when cursor on a tab).  */
             if (next->first_glyph->type != STRETCH_GLYPH)
               {
                 n = ns_get_glyph_string_clip_rect (s->next, r);
-                ns_focus (s->f, r, n);
-                ns_maybe_dumpglyphs_background (s->next, 1);
-                ns_unfocus (s->f);
+                if (ns_focus (s->f, r, n))
+                  {
+                    ns_maybe_dumpglyphs_background (s->next, 1);
+                    ns_unfocus (s->f);
+                  }
               }
             else
               {
@@ -4231,10 +4216,12 @@ overwriting cursor (usually when cursor on a tab).  */
 	    || s->first_glyph->type == COMPOSITE_GLYPH))
     {
       n = ns_get_glyph_string_clip_rect (s, r);
-      ns_focus (s->f, r, n);
-      ns_maybe_dumpglyphs_background (s, 1);
-      ns_dumpglyphs_box_or_relief (s);
-      ns_unfocus (s->f);
+      if (ns_focus (s->f, r, n))
+        {
+          ns_maybe_dumpglyphs_background (s, 1);
+          ns_dumpglyphs_box_or_relief (s);
+          ns_unfocus (s->f);
+        }
       box_drawn_p = 1;
     }
 
@@ -4243,9 +4230,11 @@ overwriting cursor (usually when cursor on a tab).  */
 
     case IMAGE_GLYPH:
       n = ns_get_glyph_string_clip_rect (s, r);
-      ns_focus (s->f, r, n);
-      ns_dumpglyphs_image (s, r[0]);
-      ns_unfocus (s->f);
+      if (ns_focus (s->f, r, n))
+        {
+          ns_dumpglyphs_image (s, r[0]);
+          ns_unfocus (s->f);
+        }
       break;
 
     case STRETCH_GLYPH:
@@ -4255,66 +4244,68 @@ overwriting cursor (usually when cursor on a tab).  */
     case CHAR_GLYPH:
     case COMPOSITE_GLYPH:
       n = ns_get_glyph_string_clip_rect (s, r);
-      ns_focus (s->f, r, n);
+      if (ns_focus (s->f, r, n))
+        {
+          if (s->for_overlaps || (s->cmp_from > 0
+                                  && ! s->first_glyph->u.cmp.automatic))
+            s->background_filled_p = 1;
+          else
+            ns_maybe_dumpglyphs_background
+              (s, s->first_glyph->type == COMPOSITE_GLYPH);
 
-      if (s->for_overlaps || (s->cmp_from > 0
-			      && ! s->first_glyph->u.cmp.automatic))
-        s->background_filled_p = 1;
-      else
-        ns_maybe_dumpglyphs_background
-          (s, s->first_glyph->type == COMPOSITE_GLYPH);
+          if (s->hl == DRAW_CURSOR && s->w->phys_cursor_type == FILLED_BOX_CURSOR)
+            {
+              unsigned long tmp = NS_FACE_BACKGROUND (s->face);
+              NS_FACE_BACKGROUND (s->face) = NS_FACE_FOREGROUND (s->face);
+              NS_FACE_FOREGROUND (s->face) = tmp;
+            }
 
-      if (s->hl == DRAW_CURSOR && s->w->phys_cursor_type == FILLED_BOX_CURSOR)
-        {
-          unsigned long tmp = NS_FACE_BACKGROUND (s->face);
-          NS_FACE_BACKGROUND (s->face) = NS_FACE_FOREGROUND (s->face);
-          NS_FACE_FOREGROUND (s->face) = tmp;
-        }
+          {
+            BOOL isComposite = s->first_glyph->type == COMPOSITE_GLYPH;
 
-      {
-        BOOL isComposite = s->first_glyph->type == COMPOSITE_GLYPH;
+            if (isComposite)
+              ns_draw_composite_glyph_string_foreground (s);
+            else
+              ns_draw_glyph_string_foreground (s);
+          }
 
-        if (isComposite)
-          ns_draw_composite_glyph_string_foreground (s);
-        else
-          ns_draw_glyph_string_foreground (s);
-      }
+          {
+            NSColor *col = (NS_FACE_FOREGROUND (s->face) != 0
+                            ? ns_lookup_indexed_color (NS_FACE_FOREGROUND (s->face),
+                                                       s->f)
+                            : FRAME_FOREGROUND_COLOR (s->f));
+            [col set];
+
+            /* Draw underline, overline, strike-through.  */
+            ns_draw_text_decoration (s, s->face, col, s->width, s->x);
+          }
 
-      {
-        NSColor *col = (NS_FACE_FOREGROUND (s->face) != 0
-                        ? ns_lookup_indexed_color (NS_FACE_FOREGROUND (s->face),
-                                                   s->f)
-                        : FRAME_FOREGROUND_COLOR (s->f));
-        [col set];
-
-        /* Draw underline, overline, strike-through.  */
-        ns_draw_text_decoration (s, s->face, col, s->width, s->x);
-      }
+          if (s->hl == DRAW_CURSOR && s->w->phys_cursor_type == FILLED_BOX_CURSOR)
+            {
+              unsigned long tmp = NS_FACE_BACKGROUND (s->face);
+              NS_FACE_BACKGROUND (s->face) = NS_FACE_FOREGROUND (s->face);
+              NS_FACE_FOREGROUND (s->face) = tmp;
+            }
 
-      if (s->hl == DRAW_CURSOR && s->w->phys_cursor_type == FILLED_BOX_CURSOR)
-        {
-          unsigned long tmp = NS_FACE_BACKGROUND (s->face);
-          NS_FACE_BACKGROUND (s->face) = NS_FACE_FOREGROUND (s->face);
-          NS_FACE_FOREGROUND (s->face) = tmp;
+          ns_unfocus (s->f);
         }
-
-      ns_unfocus (s->f);
       break;
 
     case GLYPHLESS_GLYPH:
       n = ns_get_glyph_string_clip_rect (s, r);
-      ns_focus (s->f, r, n);
-
-      if (s->for_overlaps || (s->cmp_from > 0
-			      && ! s->first_glyph->u.cmp.automatic))
-        s->background_filled_p = 1;
-      else
-        ns_maybe_dumpglyphs_background
-          (s, s->first_glyph->type == COMPOSITE_GLYPH);
-      /* ... */
-      /* Not yet implemented.  */
-      /* ... */
-      ns_unfocus (s->f);
+      if (ns_focus (s->f, r, n))
+        {
+          if (s->for_overlaps || (s->cmp_from > 0
+                                  && ! s->first_glyph->u.cmp.automatic))
+            s->background_filled_p = 1;
+          else
+            ns_maybe_dumpglyphs_background
+              (s, s->first_glyph->type == COMPOSITE_GLYPH);
+          /* ... */
+          /* Not yet implemented.  */
+          /* ... */
+          ns_unfocus (s->f);
+        }
       break;
 
     default:
@@ -4325,9 +4316,11 @@ overwriting cursor (usually when cursor on a tab).  */
   if (!s->for_overlaps && !box_drawn_p && s->face->box != FACE_NO_BOX)
     {
       n = ns_get_glyph_string_clip_rect (s, r);
-      ns_focus (s->f, r, n);
-      ns_dumpglyphs_box_or_relief (s);
-      ns_unfocus (s->f);
+      if (ns_focus (s->f, r, n))
+        {
+          ns_dumpglyphs_box_or_relief (s);
+          ns_unfocus (s->f);
+        }
     }
 
   s->num_clips = 0;
@@ -5133,7 +5126,7 @@ static Lisp_Object ns_string_to_lispmod (const char *s)
   ns_after_update_window_line,
   ns_update_window_begin,
   ns_update_window_end,
-  0, /* flush_display */
+  ns_flush_display, /* flush_display */
   x_clear_window_mouse_face,
   x_get_glyph_overhangs,
   x_fix_overlapping_area,
-- 
2.19.0


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

* bug#32812: 27.0.50; macOS Mojave GNU Emacs crash after 5-10 minutes or so
  2018-09-23 15:21 bug#32812: 27.0.50; macOS Mojave GNU Emacs crash after 5-10 minutes or so Zack Piper
@ 2018-09-24 10:17 ` Alan Third
  2018-09-24 10:24   ` Alan Third
  2018-09-24 11:39   ` Zack Piper
  0 siblings, 2 replies; 7+ messages in thread
From: Alan Third @ 2018-09-24 10:17 UTC (permalink / raw)
  To: Zack Piper; +Cc: 32812

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

On Sun, Sep 23, 2018 at 04:21:13PM +0100, Zack Piper wrote:
> Hi!
> 
> (Replacing my post in #31904 with this since they're separate issues(?))

Sorry, I somehow missed that previous email.

> I've applied a slightly modified (i.e. updated) version of Alan's patch from
> bug #31904
> (attached) to fix an issue concerning the buffer and mode-line not being
> rendered. The patch works great, everything is now rendered.
> 
> Now I notice that after 5-10 minutes (or even before), Emacs crashes with
> the below:
> 
> ```
> objc[82784]: Invalid or prematurely-freed autorelease pool 0x1030032e0.
> ```
> 
> This started occurring since upgrading to macOS Mojave, so possibly there's
> a bug with Mojave or just some incompatibility, not really sure!

It’s a bit strange looking, and autorelease pools are something of a
mystery to me.

I have a suspicion, though, that just creating a new autorelease pool
may solve this. I’ve attached a patch. It’s for emacs-26, but should
apply OK to master, or without too much work anyway.

It simply wraps the call to displayIfNeeded in a new pool:

static void
ns_flush_display (struct frame *f)
/* Force the frame to redisplay.  If areas have previously been marked
   dirty by setNeedsDisplayInRect (in ns_focus), then this will call
   draw_rect: which will "expose" those areas.  */
{
  NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
  [FRAME_NS_VIEW (f) displayIfNeeded];
  [pool release];
}

Thanks for testing the patch and raising this issue. There’s not been
a lot of feedback so far and I think we’re going to have to commit the
patch soon given that Mojave is out today.
-- 
Alan Third

[-- Attachment #2: 0001-Fix-crash-when-flushing-to-screen-bug-32812.patch --]
[-- Type: text/plain, Size: 829 bytes --]

From 17bab071cc15aa378e96913ea84167fc4af88f02 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Mon, 24 Sep 2018 11:02:41 +0100
Subject: [PATCH] Fix crash when flushing to screen (bug#32812)

* src/nsterm.m (ns_flush_display): Create a new autorelease pool for
the display code.
---
 src/nsterm.m | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/nsterm.m b/src/nsterm.m
index b36d847eb3..d6f6d8afee 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1252,7 +1252,9 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
    dirty by setNeedsDisplayInRect (in ns_focus), then this will call
    draw_rect: which will "expose" those areas.  */
 {
+  NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
   [FRAME_NS_VIEW (f) displayIfNeeded];
+  [pool release];
 }
 
 
-- 
2.18.0


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

* bug#32812: 27.0.50; macOS Mojave GNU Emacs crash after 5-10 minutes or so
  2018-09-24 10:17 ` Alan Third
@ 2018-09-24 10:24   ` Alan Third
  2018-09-24 11:39   ` Zack Piper
  1 sibling, 0 replies; 7+ messages in thread
From: Alan Third @ 2018-09-24 10:24 UTC (permalink / raw)
  To: Zack Piper; +Cc: 32812

On Mon, Sep 24, 2018 at 11:17:06AM +0100, Alan Third wrote:
> 
> It’s a bit strange looking, and autorelease pools are something of a
> mystery to me.
> 
> I have a suspicion, though, that just creating a new autorelease pool
> may solve this. I’ve attached a patch. It’s for emacs-26, but should
> apply OK to master, or without too much work anyway.

This patch doesn’t help, I finally reproduced the crash with it in
place.
-- 
Alan Third





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

* bug#32812: 27.0.50; macOS Mojave GNU Emacs crash after 5-10 minutes or so
  2018-09-24 10:17 ` Alan Third
  2018-09-24 10:24   ` Alan Third
@ 2018-09-24 11:39   ` Zack Piper
  2018-09-24 13:11     ` Alan Third
  1 sibling, 1 reply; 7+ messages in thread
From: Zack Piper @ 2018-09-24 11:39 UTC (permalink / raw)
  To: 32812

(Final attempt at sending, TLS was being enforced for my outgoing 
emails, sorry Alan for the spam!)

Hi Alan,


On 24 Sep 2018, at 11:17, Alan Third wrote:

> On Sun, Sep 23, 2018 at 04:21:13PM +0100, Zack Piper wrote:
>> Hi!
>>
>> (Replacing my post in #31904 with this since they're separate 
>> issues(?))
>
> Sorry, I somehow missed that previous email.

No worries at all!


>> I've applied a slightly modified (i.e. updated) version of Alan's 
>> patch from
>> bug #31904
>> (attached) to fix an issue concerning the buffer and mode-line not 
>> being
>> rendered. The patch works great, everything is now rendered.
>>
>> Now I notice that after 5-10 minutes (or even before), Emacs crashes 
>> with
>> the below:
>>
>> ```
>> objc[82784]: Invalid or prematurely-freed autorelease pool 
>> 0x1030032e0.
>> ```
>>
>> This started occurring since upgrading to macOS Mojave, so possibly 
>> there's
>> a bug with Mojave or just some incompatibility, not really sure!
>
> It’s a bit strange looking, and autorelease pools are something of a
> mystery to me.
>
> I have a suspicion, though, that just creating a new autorelease pool
> may solve this. I’ve attached a patch. It’s for emacs-26, but 
> should
> apply OK to master, or without too much work anyway.

>
> It simply wraps the call to displayIfNeeded in a new pool:
>
> static void
> ns_flush_display (struct frame *f)
> /* Force the frame to redisplay.  If areas have previously been marked
>    dirty by setNeedsDisplayInRect (in ns_focus), then this will call
>    draw_rect: which will "expose" those areas.  */
> {
>   NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
>   [FRAME_NS_VIEW (f) displayIfNeeded];
>   [pool release];
> }

>
> Thanks for testing the patch and raising this issue. There’s not 
> been
> a lot of feedback so far and I think we’re going to have to commit 
> the
> patch soon given that Mojave is out today.

You're very welcome, and thanks for providing the additional patch!


(Later email):


> This patch doesn’t help, I finally reproduced the crash with it in
> place.

Oh, damn.

I truly wish I could help somewhat, if you need anything additional from 
me let me know.

I've never really meddled Objective C before, so I'd likely not be able 
to help much.

Obviously I'm more than happy to test any patches you throw at me! :-)

I did look at the autorelease pool thing and tried to see where it would 
fit in, to no avail.

Let me know if you need anything.



> -- 
> Alan Third

Thanks,
Zack





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

* bug#32812: 27.0.50; macOS Mojave GNU Emacs crash after 5-10 minutes or so
  2018-09-24 11:39   ` Zack Piper
@ 2018-09-24 13:11     ` Alan Third
  2018-09-24 14:16       ` Zack Piper
  2018-11-30 10:17       ` Alan Third
  0 siblings, 2 replies; 7+ messages in thread
From: Alan Third @ 2018-09-24 13:11 UTC (permalink / raw)
  To: Zack Piper; +Cc: 32812

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

On Mon, Sep 24, 2018 at 12:39:22PM +0100, Zack Piper wrote:
> (Final attempt at sending, TLS was being enforced for my outgoing emails,
> sorry Alan for the spam!)

I think I actually did receive all the emails. No big deal, though.

> > This patch doesn’t help, I finally reproduced the crash with it in
> > place.
> 
> Oh, damn.

I think I’ve got it this time. I haven’t managed to reproduce the
crash, but I may just have been lucky.

> I truly wish I could help somewhat, if you need anything additional from me
> let me know.

That’s OK, just testing the patches is helpful.

-- 
Alan Third

[-- Attachment #2: 0001-Fix-crash-on-flush-to-display-bug-32812.patch --]
[-- Type: text/plain, Size: 794 bytes --]

From 356f20b5d8b5809100102797c51f1b54cc8a908e Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Mon, 24 Sep 2018 14:04:54 +0100
Subject: [PATCH] Fix crash on flush to display (bug#32812)

* src/nsterm.m (ns_flush_display): Block input to prevent Emacs IO
processing inside AppKit code.
---
 src/nsterm.m | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/nsterm.m b/src/nsterm.m
index b36d847eb3..4c03d35c69 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1252,7 +1252,9 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
    dirty by setNeedsDisplayInRect (in ns_focus), then this will call
    draw_rect: which will "expose" those areas.  */
 {
+  block_input ();
   [FRAME_NS_VIEW (f) displayIfNeeded];
+  unblock_input ();
 }
 
 
-- 
2.18.0


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

* bug#32812: 27.0.50; macOS Mojave GNU Emacs crash after 5-10 minutes or so
  2018-09-24 13:11     ` Alan Third
@ 2018-09-24 14:16       ` Zack Piper
  2018-11-30 10:17       ` Alan Third
  1 sibling, 0 replies; 7+ messages in thread
From: Zack Piper @ 2018-09-24 14:16 UTC (permalink / raw)
  To: Alan Third; +Cc: 32812

Hi Alan,

On 24 Sep 2018, at 14:11, Alan Third wrote:

> On Mon, Sep 24, 2018 at 12:39:22PM +0100, Zack Piper wrote:
>> (Final attempt at sending, TLS was being enforced for my outgoing 
>> emails,
>> sorry Alan for the spam!)
>
> I think I actually did receive all the emails. No big deal, though.
>

I forgot to take you out of the CC when trying to get the email to go to 
debbugs

Seems debbugs.gnu.org SMTP doesn't offer TLS, according to my mail 
server
at least.

I just disabled the setting to strictly enforce TLS outbound.

>>> This patch doesn’t help, I finally reproduced the crash with it in
>>> place.
>>
>> Oh, damn.
>
> I think I’ve got it this time. I haven’t managed to reproduce the
> crash, but I may just have been lucky.
>

It's been an hour of use and it hasn't crashed. I can't thank you 
enough!

>> I truly wish I could help somewhat, if you need anything additional 
>> from me
>> let me know.
>
> That’s OK, just testing the patches is helpful.
>
> -- 
> Alan Third

Thanks,
Zack





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

* bug#32812: 27.0.50; macOS Mojave GNU Emacs crash after 5-10 minutes or so
  2018-09-24 13:11     ` Alan Third
  2018-09-24 14:16       ` Zack Piper
@ 2018-11-30 10:17       ` Alan Third
  1 sibling, 0 replies; 7+ messages in thread
From: Alan Third @ 2018-11-30 10:17 UTC (permalink / raw)
  To: Zack Piper; +Cc: 32812-done

Alan Third <alan@idiocy.org> writes:

> On Mon, Sep 24, 2018 at 12:39:22PM +0100, Zack Piper wrote:
>> (Final attempt at sending, TLS was being enforced for my outgoing emails,
>> sorry Alan for the spam!)
>
> I think I actually did receive all the emails. No big deal, though.
>
>> > This patch doesn’t help, I finally reproduced the crash with it in
>> > place.
>> 
>> Oh, damn.
>
> I think I’ve got it this time. I haven’t managed to reproduce the
> crash, but I may just have been lucky.
>
>> I truly wish I could help somewhat, if you need anything additional from me
>> let me know.
>
> That’s OK, just testing the patches is helpful.

The code that was causing this crash has been completely removed now, so
I'm closing this bug report.
-- 
Alan Third





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

end of thread, other threads:[~2018-11-30 10:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-23 15:21 bug#32812: 27.0.50; macOS Mojave GNU Emacs crash after 5-10 minutes or so Zack Piper
2018-09-24 10:17 ` Alan Third
2018-09-24 10:24   ` Alan Third
2018-09-24 11:39   ` Zack Piper
2018-09-24 13:11     ` Alan Third
2018-09-24 14:16       ` Zack Piper
2018-11-30 10:17       ` Alan Third

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).