Skip to content

fix(datagrid): resolve header comments from the header view to stop a sort crash (#1869)#1870

Merged
datlechin merged 1 commit into
mainfrom
fix/1869-header-cell-shallow-copy-crash
Jul 14, 2026
Merged

fix(datagrid): resolve header comments from the header view to stop a sort crash (#1869)#1870
datlechin merged 1 commit into
mainfrom
fix/1869-header-cell-shallow-copy-crash

Conversation

@datlechin

Copy link
Copy Markdown
Member

Fixes #1869.

Clicking a column header crashes the app with EXC_BAD_ACCESS (SIGSEGV) at a garbage address, inside -[NSTextFieldCell dealloc] during NSDisplayCycleFlush. It reproduces on any table whose last column has a comment longer than 15 UTF-8 bytes.

Root cause

SortableHeaderCell is the repo's only NSCell subclass. In 0.56.2 (#1842) it gained headerComment: String?, its first reference-counted stored property.

NSCell.copyWithZone: is a legacy NSCopyObject shallow copy: it bit-copies subclass ivars and re-retains only the ones the Objective-C runtime can see. class_getIvarLayout returns NULL for Swift classes, so the Swift ivar is pointer-copied with no retain, while the copy's ARC .cxx_destruct still releases it on dealloc.

AppKit copies the header cell while drawing (NSTableHeaderView._preparedHeaderFillerCell, reached from drawRect: and from vibrancy updates). A header click dirties the header, the copy is made and autoreleased, and it dies at objc_autoreleasePoolPop inside the display cycle, releasing a string it never retained. The real cell is then left pointing at freed memory.

Comments shorter than 16 UTF-8 bytes live inline in the String with no heap object, which is why this only fires on tables with real column comments. Apple's NSTableColumn docs require a custom headerCell to "properly implement -copyWithZone:"; we never did.

This is live in 0.57.0 as well, and #1863 makes comments populate earlier, so it fires more readily there than in 0.56.2.

The fix

Rather than compensating for the broken copy with a retain, the comment moves off the cell entirely, so the bug class is gone:

  • SortableHeaderCell now holds only trivial state (a raw-value enum, Int?, Bools). With nothing reference-counted, NSCopyObject has nothing to get wrong no matter how often AppKit copies it.
  • SortableHeaderView (an NSView, not subject to NSCopyObject) owns the comments keyed by column identifier. The cell resolves its comment from controlView at draw time.
  • DataGridColumnPool publishes comments to the header view, and now bakes the comment into the header's accessibility label (it already built the tooltip from the same value), so moving the comment off the cell costs no accessibility.

A copied cell belongs to no column, so it resolves no comment and the overflow filler cell cannot draw a stray one.

Tests

The existing tests could not have caught this: they assigned comments from string literals, which are immortal and never reference-counted. The new tests build the comment at runtime so a real heap object is exercised, then copy the cell the way AppKit does.

  • header comment resolves from the header view, not the cell
  • a copied cell resolves no comment (overflow filler draws none)
  • the cell survives the shallow copies AppKit makes while drawing

All 26 tests across SortableHeaderCellTests and DataGridColumnPoolTests pass; swiftlint lint --strict is clean on the changed files.

Verification note

Tests cover comment resolution and copy safety, but not drawing. Worth one manual pass on a table with commented columns to confirm the comment still renders in the header and the header still grows to fit it.

https://claude.ai/code/session_01P7qP2VjoTKfq4Nh19roxtu

@datlechin datlechin merged commit f579fbe into main Jul 14, 2026
3 checks passed
@datlechin datlechin deleted the fix/1869-header-cell-shallow-copy-crash branch July 14, 2026 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Crash] App crashes with EXC_BAD_ACCESS (SIGSEGV) in Thread 0 during UI dealloc

1 participant