From d016482c86e403a64c6650c5190061e70f7703e5 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Sat, 30 May 2026 16:40:55 +0200 Subject: [PATCH 1/2] GhosttyHost: guard the remaining native callbacks against exceptions crossing the boundary OnAction was hardened against managed exceptions unwinding across the unmanaged ABI into Zig, but the other callbacks libghostty invokes directly on its own thread had the same exposure: - OnCloseSurface calls GCHandle.FromIntPtr, which throws on a bad handle. - OnConfirmReadClipboard decodes a raw C string (Marshal.PtrToStringUTF8). - OnWriteClipboard walks native memory via the content marshaller. - OnReadClipboard and OnWakeup share the same boundary. Wrap each synchronous body in a try/catch that logs and absorbs the exception (OnReadClipboard returns 0 = "not handled"). The clipboard bridge's deferred dispatcher work already self-guards on the UI thread, so only the native-thread portion needed protecting here. --- windows/Ghostty/Hosting/GhosttyHost.cs | 80 +++++++++++++++++++++----- 1 file changed, 65 insertions(+), 15 deletions(-) diff --git a/windows/Ghostty/Hosting/GhosttyHost.cs b/windows/Ghostty/Hosting/GhosttyHost.cs index 2c134bf494..799ddcc1eb 100644 --- a/windows/Ghostty/Hosting/GhosttyHost.cs +++ b/windows/Ghostty/Hosting/GhosttyHost.cs @@ -355,12 +355,20 @@ public void Dispose() private void OnWakeup(IntPtr userdata) { - // Fires on libghostty's thread. Hop to the UI dispatcher so the - // tick (and any resulting draws) lands on the right queue. - _dispatcher.TryEnqueue(() => + // Native callback on libghostty's thread (see OnAction for why the + // boundary is guarded). Hop to the UI dispatcher so the tick and any + // resulting draws land on the right queue. + try + { + _dispatcher.TryEnqueue(() => + { + if (_app.Handle != IntPtr.Zero) NativeMethods.AppTick(_app); + }); + } + catch (Exception ex) { - if (_app.Handle != IntPtr.Zero) NativeMethods.AppTick(_app); - }); + _logger.LogError(ex, "OnWakeup threw at the native boundary"); + } } // ghostty_target_s tag values, mirroring ghostty.h ghostty_target_tag_e. @@ -653,26 +661,68 @@ private byte OnAction(GhosttyApp _, IntPtr targetPtr, IntPtr actionPtr) } } + // The clipboard and close-surface callbacks below are also invoked + // directly by libghostty on its own thread (see OnAction for the + // native-boundary rationale). Their synchronous bodies decode raw + // pointers and handles -- PtrToStringUTF8, the content marshaller, + // GCHandle.FromIntPtr -- any of which can throw, so each guards the + // boundary and swallows (logs) rather than letting the exception cross + // the ABI into Zig. The bridge's deferred dispatcher work self-guards. private byte OnReadClipboard(IntPtr userdata, GhosttyClipboard kind, IntPtr state) - => (_clipboardBridge?.HandleRead(userdata, kind, state) ?? false) ? (byte)1 : (byte)0; + { + try + { + return (_clipboardBridge?.HandleRead(userdata, kind, state) ?? false) ? (byte)1 : (byte)0; + } + catch (Exception ex) + { + _logger.LogError(ex, "OnReadClipboard threw at the native boundary"); + return 0; + } + } private void OnConfirmReadClipboard(IntPtr userdata, IntPtr str, IntPtr state, GhosttyClipboardRequest request) - => _clipboardBridge?.HandleConfirm(userdata, str, state, request); + { + try + { + _clipboardBridge?.HandleConfirm(userdata, str, state, request); + } + catch (Exception ex) + { + _logger.LogError(ex, "OnConfirmReadClipboard threw at the native boundary"); + } + } private void OnWriteClipboard(IntPtr userdata, GhosttyClipboard kind, IntPtr content, UIntPtr count, byte confirm) - => _clipboardBridge?.HandleWrite(userdata, kind, content, count, confirm != 0); + { + try + { + _clipboardBridge?.HandleWrite(userdata, kind, content, count, confirm != 0); + } + catch (Exception ex) + { + _logger.LogError(ex, "OnWriteClipboard threw at the native boundary"); + } + } private void OnCloseSurface(IntPtr userdata, byte processAlive) { - if (userdata == IntPtr.Zero) return; + try + { + if (userdata == IntPtr.Zero) return; - var control = GCHandle.FromIntPtr(userdata).Target as TerminalControl; - if (control is null) return; - if (!IsRegistered(control)) return; - _dispatcher.TryEnqueue(() => + var control = GCHandle.FromIntPtr(userdata).Target as TerminalControl; + if (control is null) return; + if (!IsRegistered(control)) return; + _dispatcher.TryEnqueue(() => + { + if (IsRegistered(control)) control.RaiseCloseRequested(); + }); + } + catch (Exception ex) { - if (IsRegistered(control)) control.RaiseCloseRequested(); - }); + _logger.LogError(ex, "OnCloseSurface threw at the native boundary"); + } } private bool IsRegistered(TerminalControl control) From 2a8923a23dfe95cec5b942d3640ad3da8b175816 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Sat, 30 May 2026 16:49:50 +0200 Subject: [PATCH 2/2] GhosttyHost: explain why OnReadClipboard's boundary catch returns 0 safely Review follow-up. Document that returning 0 ("not handled") cannot strand a clipboard request, since HandleRead only throws on its synchronous prefix before it takes on the completion obligation. --- windows/Ghostty/Hosting/GhosttyHost.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/windows/Ghostty/Hosting/GhosttyHost.cs b/windows/Ghostty/Hosting/GhosttyHost.cs index 799ddcc1eb..c46acc0ab6 100644 --- a/windows/Ghostty/Hosting/GhosttyHost.cs +++ b/windows/Ghostty/Hosting/GhosttyHost.cs @@ -676,6 +676,10 @@ private byte OnReadClipboard(IntPtr userdata, GhosttyClipboard kind, IntPtr stat } catch (Exception ex) { + // Returning 0 ("not handled") here cannot strand a request: + // HandleRead only throws on its synchronous prefix, before it + // returns true and takes on the obligation to complete the + // clipboard request via SurfaceCompleteClipboardRequest. _logger.LogError(ex, "OnReadClipboard threw at the native boundary"); return 0; }