From 8c16116cc948ccd8f6cc48fd7cf5db535ac5070f Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Sun, 29 Mar 2026 20:11:18 +0300 Subject: [PATCH 1/3] fix: prevent EXC_BAD_ACCESS crash on Cmd+Z undo (#650) Race condition between undo manager and async syntax highlighting: when undo/redo modifies NSTextStorage, the debounced highlighting could also call beginEditing/endEditing on the same textStorage, causing concurrent mutation and EXC_BAD_ACCESS. Fix: detect isUndoing/isRedoing in textDidChange and defer syntax highlighting until the undo manager completes. Also add guards in SyntaxHighlighter.applyMatches() and resetAttributes() to bail out if called during an active undo/redo operation. --- Pine/CodeEditorView.swift | 69 ++++++- Pine/SyntaxHighlighter.swift | 16 ++ PineTests/UndoHighlightSafetyTests.swift | 231 +++++++++++++++++++++++ 3 files changed, 314 insertions(+), 2 deletions(-) create mode 100644 PineTests/UndoHighlightSafetyTests.swift diff --git a/Pine/CodeEditorView.swift b/Pine/CodeEditorView.swift index df2476c..972ff31 100644 --- a/Pine/CodeEditorView.swift +++ b/Pine/CodeEditorView.swift @@ -1037,14 +1037,17 @@ struct CodeEditorView: NSViewRepresentable { range editedRange: NSRange, changeInLength delta: Int ) { - // Only capture character edits from user typing (not attribute-only - // changes from highlighting, and not programmatic text replacement). if editedMask.contains(.editedCharacters), !isProgrammaticTextChange { pendingEditedRange = editedRange pendingChangeInLength = delta } } + /// True while undo/redo is in progress. Prevents syntax highlighting + /// from modifying NSTextStorage attributes concurrently with the undo + /// manager's grouped operations, which causes EXC_BAD_ACCESS (#650). + private(set) var isUndoRedoInProgress = false + func textDidChange(_ notification: Notification) { guard let textView = notification.object as? NSTextView else { return } @@ -1062,6 +1065,15 @@ struct CodeEditorView: NSViewRepresentable { return } + // Detect undo/redo in progress. When the undo manager is unwinding + // grouped operations, modifying NSTextStorage attributes (via syntax + // highlighting beginEditing/endEditing) can cause a race condition + // leading to EXC_BAD_ACCESS. We defer highlighting until the undo + // manager finishes its current operation (#650). + let undoing = textView.undoManager?.isUndoing == true + let redoing = textView.undoManager?.isRedoing == true + isUndoRedoInProgress = undoing || redoing + // Mark that this change originated from the user typing, // so the upcoming updateNSView won't overwrite the text and reset the cursor. didChangeFromTextView = true @@ -1107,6 +1119,53 @@ struct CodeEditorView: NSViewRepresentable { // сдвигает символьные смещения, старый диапазон некорректен highlightedCharRange = nil + // During undo/redo, cancel any pending highlight and skip scheduling + // a new one. The undo manager may still be processing grouped + // operations — modifying textStorage attributes now would cause + // EXC_BAD_ACCESS. We schedule a deferred highlight after the undo + // manager finishes (on the next run loop iteration). + if isUndoRedoInProgress { + highlightWorkItem?.cancel() + highlightTask?.cancel() + let deferredItem = DispatchWorkItem { [weak self] in + guard let self else { return } + self.isUndoRedoInProgress = false + guard let sv = self.scrollView, + let tv = sv.documentView as? NSTextView, + let storage = tv.textStorage else { return } + // Full re-highlight after undo/redo completes safely + self.highlightGeneration.increment() + let gen = self.highlightGeneration + let lang = self.parent.language + let name = self.parent.fileName + let font = self.parent.editorFont + let isLargeFile = storage.length > CodeEditorView.viewportHighlightThreshold + if isLargeFile { + self.scheduleViewportHighlighting(textView: tv) + } else { + self.highlightTask = Task { @MainActor [weak self] in + let result = await SyntaxHighlighter.shared.highlightAsync( + textStorage: storage, + language: lang, + fileName: name, + font: font, + generation: gen + ) + if let result { + self?.parent.onHighlightCacheUpdate?(result) + } + } + } + } + highlightWorkItem = deferredItem + // Defer to next run loop — undo manager will have finished by then + DispatchQueue.main.asyncAfter( + deadline: .now() + highlightDelay * 2, + execute: deferredItem + ) + return + } + // Дебаунсинг: откладываем подсветку до паузы в вводе. // Не накапливаем диапазоны — каждый textDidChange работает // в своих координатах; union между версиями некорректен. @@ -1121,6 +1180,12 @@ struct CodeEditorView: NSViewRepresentable { let tv = sv.documentView as? NSTextView, let storage = tv.textStorage else { return } + // Double-check: if an undo/redo started between scheduling and + // execution, bail out to avoid the same race condition. + if tv.undoManager?.isUndoing == true || tv.undoManager?.isRedoing == true { + return + } + self.highlightGeneration.increment() let gen = self.highlightGeneration let lang = self.parent.language diff --git a/Pine/SyntaxHighlighter.swift b/Pine/SyntaxHighlighter.swift index c11567d..fe51385 100644 --- a/Pine/SyntaxHighlighter.swift +++ b/Pine/SyntaxHighlighter.swift @@ -586,6 +586,7 @@ final class SyntaxHighlighter: @unchecked Sendable { /// Сбрасывает атрибуты на базовый стиль (без грамматики). /// Clamps range to textStorage.length to avoid crash if text changed. + /// Skips if undo/redo is in progress to prevent EXC_BAD_ACCESS (#650). private func resetAttributes(textStorage: NSTextStorage, range: NSRange, font: NSFont) { let currentLength = textStorage.length guard currentLength > 0 else { return } @@ -596,6 +597,12 @@ final class SyntaxHighlighter: @unchecked Sendable { guard safeRange.length > 0 else { return } let undoManager = textStorage.layoutManagers.first?.firstTextView?.undoManager + + // Bail out if undo/redo is in progress (#650). + if undoManager?.isUndoing == true || undoManager?.isRedoing == true { + return + } + undoManager?.disableUndoRegistration() defer { undoManager?.enableUndoRegistration() } textStorage.beginEditing() @@ -732,6 +739,8 @@ final class SyntaxHighlighter: @unchecked Sendable { /// Applies pre-computed matches to NSTextStorage. Must be called on main thread. /// Validates that ranges are still valid — text may have changed between /// computation and application. + /// Skips application if the undo manager is currently undoing/redoing to + /// prevent EXC_BAD_ACCESS from concurrent NSTextStorage mutations (#650). func applyMatches( _ result: HighlightMatchResult, to textStorage: NSTextStorage, @@ -744,6 +753,13 @@ final class SyntaxHighlighter: @unchecked Sendable { } let undoManager = textStorage.layoutManagers.first?.firstTextView?.undoManager + + // Bail out if undo/redo is in progress — modifying textStorage attributes + // during an undo group causes EXC_BAD_ACCESS (#650). + if undoManager?.isUndoing == true || undoManager?.isRedoing == true { + return + } + undoManager?.disableUndoRegistration() defer { undoManager?.enableUndoRegistration() } diff --git a/PineTests/UndoHighlightSafetyTests.swift b/PineTests/UndoHighlightSafetyTests.swift new file mode 100644 index 0000000..a4d0161 --- /dev/null +++ b/PineTests/UndoHighlightSafetyTests.swift @@ -0,0 +1,231 @@ +// +// UndoHighlightSafetyTests.swift +// PineTests +// +// Tests for undo/redo safety during syntax highlighting (#650). +// Verifies that applyMatches and resetAttributes bail out when +// the undo manager is in the middle of undoing/redoing, preventing +// EXC_BAD_ACCESS from concurrent NSTextStorage mutations. + +import Testing +import AppKit +import SwiftUI +@testable import Pine + +@Suite("Undo Highlight Safety") +struct UndoHighlightSafetyTests { + + private let font = NSFont.monospacedSystemFont(ofSize: 13, weight: .regular) + + /// Builds a minimal text system stack with an undo manager. + private func makeTextStack(text: String) -> (NSScrollView, GutterTextView, NSTextStorage) { + let textStorage = NSTextStorage(string: text) + let layoutManager = NSLayoutManager() + textStorage.addLayoutManager(layoutManager) + let textContainer = NSTextContainer( + containerSize: NSSize(width: 500, height: CGFloat.greatestFiniteMagnitude) + ) + textContainer.widthTracksTextView = true + layoutManager.addTextContainer(textContainer) + let textView = GutterTextView( + frame: NSRect(x: 0, y: 0, width: 500, height: 500), + textContainer: textContainer + ) + textView.allowsUndo = true + let scrollView = NSScrollView(frame: NSRect(x: 0, y: 0, width: 500, height: 500)) + scrollView.documentView = textView + return (scrollView, textView, textStorage) + } + + private func makeCoordinator( + text: String = "func hello() { }", + language: String = "swift", + fileName: String? = "test.swift" + ) -> (CodeEditorView.Coordinator, NSScrollView, GutterTextView) { + let (scrollView, textView, _) = makeTextStack(text: text) + let editorView = CodeEditorView( + text: .constant(text), + contentVersion: 0, + language: language, + fileName: fileName, + foldState: .constant(FoldState()) + ) + let coordinator = CodeEditorView.Coordinator(parent: editorView) + coordinator.scrollView = scrollView + coordinator.syncContentVersion() + coordinator.lastFontSize = font.pointSize + coordinator.updateContentIfNeeded( + text: text, language: language, fileName: fileName, font: font + ) + return (coordinator, scrollView, textView) + } + + // MARK: - applyMatches safety + + @Test func applyMatches_appliesNormally_whenNoUndoInProgress() { + let text = "let x = 42" + let (_, _, textStorage) = makeTextStack(text: text) + + let match = HighlightMatch( + range: NSRange(location: 0, length: 3), + scope: "keyword", + priority: 0 + ) + let result = HighlightMatchResult( + matches: [match], + repaintRange: NSRange(location: 0, length: textStorage.length), + multilineFingerprint: [] + ) + + // Apply highlights — should succeed (no undo in progress) + SyntaxHighlighter.shared.applyMatches(result, to: textStorage, font: font) + + // Verify "let" got keyword color (not default textColor) + var effectiveRange = NSRange() + let color = textStorage.attribute( + .foregroundColor, at: 0, effectiveRange: &effectiveRange + ) as? NSColor + #expect(color != nil, "Keyword should have a color applied") + } + + @Test func applyMatches_skipsWhenRepaintRangeExceedsLength() { + let text = "short" + let (_, _, textStorage) = makeTextStack(text: text) + + let result = HighlightMatchResult( + matches: [], + repaintRange: NSRange(location: 0, length: 999), + multilineFingerprint: [] + ) + + // Should not crash — just skip + SyntaxHighlighter.shared.applyMatches(result, to: textStorage, font: font) + } + + // MARK: - Coordinator undo/redo detection + + @Test func coordinator_createsWithUndoRedoFlagFalse() { + let (coordinator, _, _) = makeCoordinator() + // The coordinator should start with undo/redo not in progress. + // We can't directly check the private isUndoRedoInProgress flag, + // but we verify that textDidChange works normally (no undo skip path). + #expect(coordinator.didChangeFromTextView == false) + } + + @Test func textDidChange_setsDidChangeFromTextView() { + let text = "hello" + let (coordinator, scrollView, textView) = makeCoordinator(text: text) + + // Simulate a text change via NSTextView + textView.string = "hello world" + + let notification = Notification( + name: NSText.didChangeNotification, + object: textView + ) + + coordinator.textDidChange(notification) + + #expect(coordinator.didChangeFromTextView == true) + } + + @Test func textDidChange_updatesParentText() { + var capturedText = "hello" + let editorView = CodeEditorView( + text: .init(get: { capturedText }, set: { capturedText = $0 }), + contentVersion: 0, + language: "swift", + fileName: "test.swift", + foldState: .constant(FoldState()) + ) + + let (scrollView, textView, _) = makeTextStack(text: "hello") + let coordinator = CodeEditorView.Coordinator(parent: editorView) + coordinator.scrollView = scrollView + coordinator.syncContentVersion() + + textView.string = "changed" + + let notification = Notification( + name: NSText.didChangeNotification, + object: textView + ) + + coordinator.textDidChange(notification) + + #expect(capturedText == "changed", + "Parent text binding should be updated after textDidChange") + } + + // MARK: - HighlightGeneration + + @Test func highlightGeneration_incrementsCorrectly() { + let gen = HighlightGeneration() + let initial = gen.current + gen.increment() + #expect(gen.current == initial + 1) + gen.increment() + #expect(gen.current == initial + 2) + } + + @Test func cancelPendingHighlight_incrementsGeneration() { + let (coordinator, _, _) = makeCoordinator() + let genBefore = coordinator.highlightGeneration.current + coordinator.cancelPendingHighlight() + #expect(coordinator.highlightGeneration.current == genBefore + 1) + } + + // MARK: - applyMatches with valid ranges + + @Test func applyMatches_appliesMultipleScopes() { + let text = "let x = \"hello\"" + let (_, _, textStorage) = makeTextStack(text: text) + + let matches = [ + HighlightMatch(range: NSRange(location: 0, length: 3), scope: "keyword", priority: 0), + HighlightMatch(range: NSRange(location: 8, length: 7), scope: "string", priority: 90), + ] + let result = HighlightMatchResult( + matches: matches, + repaintRange: NSRange(location: 0, length: textStorage.length), + multilineFingerprint: [] + ) + + SyntaxHighlighter.shared.applyMatches(result, to: textStorage, font: font) + + // Keyword color at position 0 + let kwColor = textStorage.attribute(.foregroundColor, at: 0, effectiveRange: nil) as? NSColor + #expect(kwColor != nil) + + // String color at position 8 + let strColor = textStorage.attribute(.foregroundColor, at: 8, effectiveRange: nil) as? NSColor + #expect(strColor != nil) + + // The two colors should be different (keyword vs string) + if let kw = kwColor, let str = strColor { + #expect(kw != str, "Keyword and string should have different colors") + } + } + + @Test func applyMatches_skipsMatchBeyondTextLength() { + let text = "hi" + let (_, _, textStorage) = makeTextStack(text: text) + + let matches = [ + HighlightMatch(range: NSRange(location: 0, length: 2), scope: "keyword", priority: 0), + // This match is out of bounds — should be silently skipped + HighlightMatch(range: NSRange(location: 10, length: 5), scope: "string", priority: 90), + ] + let result = HighlightMatchResult( + matches: matches, + repaintRange: NSRange(location: 0, length: textStorage.length), + multilineFingerprint: [] + ) + + // Should not crash + SyntaxHighlighter.shared.applyMatches(result, to: textStorage, font: font) + + let color = textStorage.attribute(.foregroundColor, at: 0, effectiveRange: nil) as? NSColor + #expect(color != nil, "Valid match should still be applied") + } +} From 545ad24a9fd422330ee5895b0f9248420033b879 Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Sun, 29 Mar 2026 20:23:41 +0300 Subject: [PATCH 2/3] fix: reset undo flag on each textDidChange, extract scheduleDeferredHighlight, real undo test --- Pine/CodeEditorView.swift | 81 +++++++++++------------- PineTests/UndoHighlightSafetyTests.swift | 69 ++++++++++++++++++++ 2 files changed, 105 insertions(+), 45 deletions(-) diff --git a/Pine/CodeEditorView.swift b/Pine/CodeEditorView.swift index 972ff31..815e504 100644 --- a/Pine/CodeEditorView.swift +++ b/Pine/CodeEditorView.swift @@ -1051,6 +1051,11 @@ struct CodeEditorView: NSViewRepresentable { func textDidChange(_ notification: Notification) { guard let textView = notification.object as? NSTextView else { return } + // Always reset at the start of every textDidChange — prevents the flag + // from "sticking" if a previous deferred highlightWorkItem was cancelled + // before it could clear the flag (#650 review). + isUndoRedoInProgress = false + // When text was replaced programmatically by updateContentIfNeeded, // skip highlight scheduling — updateContentIfNeeded handles its own // full highlight. Only update caches that it doesn't handle. @@ -1119,50 +1124,12 @@ struct CodeEditorView: NSViewRepresentable { // сдвигает символьные смещения, старый диапазон некорректен highlightedCharRange = nil - // During undo/redo, cancel any pending highlight and skip scheduling - // a new one. The undo manager may still be processing grouped - // operations — modifying textStorage attributes now would cause - // EXC_BAD_ACCESS. We schedule a deferred highlight after the undo - // manager finishes (on the next run loop iteration). + // During undo/redo, cancel any pending highlight and schedule a + // deferred full re-highlight. The undo manager may still be processing + // grouped operations — modifying textStorage attributes now would cause + // EXC_BAD_ACCESS (#650). if isUndoRedoInProgress { - highlightWorkItem?.cancel() - highlightTask?.cancel() - let deferredItem = DispatchWorkItem { [weak self] in - guard let self else { return } - self.isUndoRedoInProgress = false - guard let sv = self.scrollView, - let tv = sv.documentView as? NSTextView, - let storage = tv.textStorage else { return } - // Full re-highlight after undo/redo completes safely - self.highlightGeneration.increment() - let gen = self.highlightGeneration - let lang = self.parent.language - let name = self.parent.fileName - let font = self.parent.editorFont - let isLargeFile = storage.length > CodeEditorView.viewportHighlightThreshold - if isLargeFile { - self.scheduleViewportHighlighting(textView: tv) - } else { - self.highlightTask = Task { @MainActor [weak self] in - let result = await SyntaxHighlighter.shared.highlightAsync( - textStorage: storage, - language: lang, - fileName: name, - font: font, - generation: gen - ) - if let result { - self?.parent.onHighlightCacheUpdate?(result) - } - } - } - } - highlightWorkItem = deferredItem - // Defer to next run loop — undo manager will have finished by then - DispatchQueue.main.asyncAfter( - deadline: .now() + highlightDelay * 2, - execute: deferredItem - ) + scheduleDeferredHighlight(editedRange: nil) return } @@ -1171,11 +1138,30 @@ struct CodeEditorView: NSViewRepresentable { // в своих координатах; union между версиями некорректен. // При быстром вводе последовательные правки обычно смежны, // и 20-строчный контекст в highlightEdited покрывает их. + scheduleDeferredHighlight(editedRange: editedRange) + } + + /// Cancels any in-flight highlight work and schedules a new debounced + /// highlight pass. When `editedRange` is non-nil, an incremental + /// `highlightEditedAsync` is attempted first; otherwise a full + /// re-highlight runs. + /// + /// Called from both normal edits and undo/redo paths to avoid + /// duplicating the scheduling logic. + private func scheduleDeferredHighlight(editedRange: NSRange?) { highlightWorkItem?.cancel() highlightTask?.cancel() - let isLargeFile = (textView.string as NSString).length > CodeEditorView.viewportHighlightThreshold + + let isUndoRedo = isUndoRedoInProgress + let workItem = DispatchWorkItem { [weak self] in guard let self else { return } + + // Clear the undo/redo flag now that we're past the danger zone. + if isUndoRedo { + self.isUndoRedoInProgress = false + } + guard let sv = self.scrollView, let tv = sv.documentView as? NSTextView, let storage = tv.textStorage else { return } @@ -1191,6 +1177,7 @@ struct CodeEditorView: NSViewRepresentable { let lang = self.parent.language let name = self.parent.fileName let font = self.parent.editorFont + let isLargeFile = storage.length > CodeEditorView.viewportHighlightThreshold if let range = editedRange, range.location + range.length <= storage.length { self.highlightTask = Task { @MainActor in @@ -1221,7 +1208,11 @@ struct CodeEditorView: NSViewRepresentable { } } highlightWorkItem = workItem - DispatchQueue.main.asyncAfter(deadline: .now() + highlightDelay, execute: workItem) + // During undo/redo, dispatch on next run loop iteration so the undo + // manager finishes its grouped operations before we touch textStorage. + // Normal edits use the standard debounce delay. + let delay: TimeInterval = isUndoRedo ? 0 : highlightDelay + DispatchQueue.main.asyncAfter(deadline: .now() + delay, execute: workItem) } func textViewDidChangeSelection(_ notification: Notification) { diff --git a/PineTests/UndoHighlightSafetyTests.swift b/PineTests/UndoHighlightSafetyTests.swift index a4d0161..99dbdcf 100644 --- a/PineTests/UndoHighlightSafetyTests.swift +++ b/PineTests/UndoHighlightSafetyTests.swift @@ -228,4 +228,73 @@ struct UndoHighlightSafetyTests { let color = textStorage.attribute(.foregroundColor, at: 0, effectiveRange: nil) as? NSColor #expect(color != nil, "Valid match should still be applied") } + + // MARK: - Real undoManager.undo() integration + + @Test func applyMatches_skipsWhenRealUndoManagerIsUndoing() { + let text = "let x = 42" + let (_, textView, textStorage) = makeTextStack(text: text) + + // Register a real undo action so undoManager.undo() triggers isUndoing + textView.undoManager?.registerUndo(withTarget: textView) { tv in + tv.string = "let x = 42" + } + textView.undoManager?.setActionName("Test Edit") + + // Build a highlight result that would color "let" as a keyword + let match = HighlightMatch( + range: NSRange(location: 0, length: 3), + scope: "keyword", + priority: 0 + ) + let result = HighlightMatchResult( + matches: [match], + repaintRange: NSRange(location: 0, length: textStorage.length), + multilineFingerprint: [] + ) + + // Record the default foreground color before any highlighting + let colorBefore = textStorage.attribute( + .foregroundColor, at: 0, effectiveRange: nil + ) as? NSColor + + // Trigger undo — inside the undo block, isUndoing == true + // We hijack the undo action to call applyMatches mid-undo + textView.undoManager?.registerUndo(withTarget: textView) { [font] _ in + // This runs while isUndoing == true + SyntaxHighlighter.shared.applyMatches(result, to: textStorage, font: font) + } + textView.undoManager?.undo() + + // applyMatches should have bailed out — foreground color must be unchanged + let colorAfter = textStorage.attribute( + .foregroundColor, at: 0, effectiveRange: nil + ) as? NSColor + #expect( + colorBefore == colorAfter, + "applyMatches must not apply highlights while undoManager.isUndoing == true" + ) + } + + @Test func isUndoRedoInProgress_resetsOnNextTextDidChange() { + let (coordinator, _, textView) = makeCoordinator(text: "hello") + + // Simulate that isUndoRedoInProgress got stuck (e.g., deferred work item cancelled) + // We need to set it via textDidChange with an undoing undoManager first. + // Instead, since we made it private(set), we verify the reset behavior: + // 1. Trigger textDidChange with no undo in progress + // 2. Verify the flag is false + textView.string = "hello world" + let notification = Notification( + name: NSText.didChangeNotification, + object: textView + ) + coordinator.textDidChange(notification) + + // After a normal textDidChange, isUndoRedoInProgress should be false + #expect( + coordinator.isUndoRedoInProgress == false, + "isUndoRedoInProgress must be reset at the start of textDidChange" + ) + } } From 69c8b1656ffda82a981de19ed4cb37c744981feb Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Sun, 29 Mar 2026 20:31:55 +0300 Subject: [PATCH 3/3] fix: restore Homebrew badge link to homebrew-tap repo (#651) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f1abec7..01681af 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ [![CI](https://github.com/batonogov/pine/actions/workflows/ci.yml/badge.svg?branch=main)](https://github.com/batonogov/pine/actions/workflows/ci.yml) [![Release](https://img.shields.io/github/v/release/batonogov/pine)](https://github.com/batonogov/pine/releases/latest) -[![Homebrew](https://img.shields.io/homebrew/cask/v/pine-editor)](https://formulae.brew.sh/cask/pine-editor) +[![Homebrew Cask](https://img.shields.io/badge/homebrew-pine--editor-orange)](https://github.com/batonogov/homebrew-tap) [![License: MIT](https://img.shields.io/github/license/batonogov/pine)](https://github.com/batonogov/pine/blob/main/LICENSE) [![macOS](https://img.shields.io/badge/macOS-26%2B-blue)](https://github.com/batonogov/pine)