Skip to content

Commit 05d5153

Browse files
Fix EXC_BAD_ACCESS crash: MPDocument.editor IBOutlet was unsafe_unretained
Reported as a crash on the Underline toolbar action, but the bug wasn't in the underline code path itself (toggleForMarkupPrefix:suffix: in NSTextView+Autocomplete.m was untouched and correct) -- it was in how MPDocument holds its reference to the editor view. 'editor' was the only IBOutlet on the class declared unsafe_unretained; every sibling outlet (toolbar, splitView, editorContainer, preview, ...) is weak. Traced via git blame to the original 2014 project setup, never updated since. unsafe_unretained is a raw, non-zeroing pointer: if the MPEditorView is ever deallocated and recreated (window/layout churn), 'editor' keeps pointing at freed memory, and the next message sent to it -- from any action, not just Underline -- is a dangling-pointer access, i.e. EXC_BAD_ACCESS. weak makes the pointer nil itself out instead, turning a crash into a safe no-op.
1 parent ae62fb1 commit 05d5153

2 files changed

Lines changed: 22 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,18 @@ compliance. The Markdown parser (Hoedown) is untouched.
141141

142142
### Fixed
143143

144+
- Crash (`EXC_BAD_ACCESS`) on toolbar formatting actions (reported on
145+
Underline, but not specific to it — any action touching `self.editor`
146+
was at risk). `MPDocument`'s `editor` IBOutlet was declared
147+
`unsafe_unretained`, the one holdout among all of its sibling outlets
148+
(`toolbar`, `splitView`, `editorContainer`, `preview`, ...), which are
149+
all `weak` — traced via `git blame` to the original 2014 project setup,
150+
predating this codebase's adoption of `weak` IBOutlets, never updated
151+
since. A raw, non-zeroing pointer means that if the underlying
152+
`MPEditorView` is ever deallocated and recreated (window/layout churn),
153+
`editor` keeps pointing at freed memory; any later message to it is a
154+
dangling-pointer access. Changed to `weak`, matching every other outlet
155+
on the class.
144156
- `Tools/update_build_number.sh`: unquoted `$(pwd -P)` broke the build when
145157
the checkout path contained a space (pre-existing bug, unrelated to this
146158
modernization pass).

MacDown/Code/Document/MPDocument.m

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,16 @@ typedef NS_ENUM(NSUInteger, MPWordCountType) {
159159
@property (weak) IBOutlet NSToolbar *toolbar;
160160
@property (weak) IBOutlet MPDocumentSplitView *splitView;
161161
@property (weak) IBOutlet NSView *editorContainer;
162-
@property (unsafe_unretained) IBOutlet MPEditorView *editor;
162+
// unsafe_unretained here (unlike every sibling IBOutlet above, all `weak`)
163+
// dates back to the original 2014 project setup, predating this codebase's
164+
// adoption of `weak` IBOutlets, and was never updated. A raw, non-zeroing
165+
// pointer means that if the underlying MPEditorView is ever deallocated
166+
// and recreated (e.g. during window/layout churn), `editor` keeps pointing
167+
// at freed memory -- any later message to it (toggleForMarkupPrefix:suffix:,
168+
// .selectedRange, .string, ...) is then a dangling-pointer access, i.e.
169+
// EXC_BAD_ACCESS. `weak` makes the pointer nil itself out instead of
170+
// dangling, turning a crash into a safe no-op.
171+
@property (weak) IBOutlet MPEditorView *editor;
163172
@property (weak) IBOutlet NSLayoutConstraint *editorPaddingBottom;
164173
// Instantiated programmatically in -windowControllerDidLoadNib: and swapped
165174
// into the split view in place of a plain NSView placeholder (see

0 commit comments

Comments
 (0)