Skip to content

Fix: Replace AsyncWorker with ThreadSafeFunction to prevent crashes#1

Merged
dacay merged 2 commits into
dacay:masterfrom
brandonkboswell:thread-safe-callbacks
Mar 19, 2026
Merged

Fix: Replace AsyncWorker with ThreadSafeFunction to prevent crashes#1
dacay merged 2 commits into
dacay:masterfrom
brandonkboswell:thread-safe-callbacks

Conversation

@brandonkboswell
Copy link
Copy Markdown
Contributor

  • Replaced CalendarAccessWorker, RemindersAccessWorker, and RemindersFetchWorker with thread-safe context structs
  • Use ThreadSafeFunction instead of AsyncWorker for cross-thread callbacks
  • Capture shared_ptr by value in lambdas to ensure promise lifetime
  • Fixes segmentation fault when macOS permission dialog callback fires

This fixes crash at memory address 0x260 when users grant calendar permissions.

 - Replaced CalendarAccessWorker, RemindersAccessWorker, and RemindersFetchWorker with thread-safe context
 structs
 - Use ThreadSafeFunction instead of AsyncWorker for cross-thread callbacks
 - Capture shared_ptr by value in lambdas to ensure promise lifetime
 - Fixes segmentation fault when macOS permission dialog callback fires

 This fixes crash at memory address 0x260 when users grant calendar permissions.
@brandonkboswell
Copy link
Copy Markdown
Contributor Author

@dacay let me start by saying I'm not an Objective-C coder or a Swift coder of any kind. I found that EventKit-node was crashing after accepting permissions when I went to use it. I had Claude code fix that issue, and the code that's in this fork seems to continue to maintain all the functionality that existed in your version, but doesn't crash after receiving proper permissions on macOS 26.2 Tahoe.

@dacay
Copy link
Copy Markdown
Owner

dacay commented Mar 19, 2026

Hey @brandonkboswell thank you the PR! Sorry to notice this, it was lost between other notifications.

While I have some production experience with Swift/Objective-C, I am not a native developer as well. This was one of my earliest AI-supported projects, so most of the low-level bindings were coded by AI.

I liked the approach Claude suggested. I got mine go over it as well, and it 2 issues that still poses an issue and some dead code after this migration. Let me share it's report with you here:

### 1. Bug (Regression) — `startDate` missing from reminders

`RemindersFetchContext::ResolveWithReminders` is missing the `startDate` field when building reminder JS objects. The old `RemindersFetchWorker::OnOK()` had it. Any code reading `reminder.startDate` from `getRemindersWithPredicate` will now get `undefined`.

### 2. Bug (Incomplete fix) — `Commit` still uses the unsafe callback pattern

The `Commit` function still directly calls `deferred.Resolve`/`deferred.Reject` from inside the Swift `commitWithCompletion:` callback block — the same thread-unsafe pattern this PR fixes elsewhere. It needs a `ThreadSafeFunction` too.

### 3. Dead code — `RemoveCalendarWorker` and `RemindersFetchWorker`

Both classes are still defined but no longer referenced after the refactor. They should be removed.

I've pushed the fixes on top of your commit on a branch and will merge from there. Thanks again!

  - Add missing startDate field in RemindersFetchContext
  - Convert Commit function to use ThreadSafeFunction
  - Remove unused RemoveCalendarWorker and RemindersFetchWorker classes

  Builds on top of brandonkboswell's AsyncWorker to ThreadSafeFunction refactor.
@dacay dacay merged commit dd28529 into dacay:master Mar 19, 2026
@dacay
Copy link
Copy Markdown
Owner

dacay commented Mar 19, 2026

Merged, thanks for the contribution!

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.

2 participants