Skip to content

fix(serial): fix SIGSEGV and file descriptor leaks in attachment constructor - #238

Merged
Code-Hex merged 5 commits into
mainfrom
codex/fix-serial-attachment-lifecycle
Sep 28, 2026
Merged

Code-Hex merged 5 commits into
mainfrom
codex/fix-serial-attachment-lifecycle

Conversation

@Code-Hex

@Code-Hex Code-Hex commented Sep 28, 2026 •

Copy link
Copy Markdown
Owner

Why

Following merged PR #217, NewFileHandleSerialPortAttachment crashed with SIGSEGV because an invalid read or write descriptor caused an autoreleased NSError to be used by Go after its Objective-C autorelease pool drained. Additionally, two duplicated file descriptors leaked because locally allocated NSFileHandle objects were not released after attachment initialization retained them. Finally, CI full VM tests were being cancelled at 3 and 5 minutes despite making progress.

Scope

  • The Objective-C bridge now retains the NSError across the pool boundary; Go converts it to a plain Go error and releases the retained NSError.
  • The bridge releases both NSFileHandles on success, and releases the first if the second duplication fails.
  • Added tests in serial_console_test.go to cover invalid read and write handles yielding EBADF and no attachment, and to ensure absence of descriptor leaks on failure and after success release.
  • Added a test helper to count /dev/fd names via open directory Readdirnames, avoiding os.ReadDir which caused fstatat bad file descriptor errors on macOS 15 Intel.
  • Increased the CI test job timeout in .github/workflows/compile.yml from 3 to 10 minutes.

Blast Radius

Changes are scoped entirely to NewFileHandleSerialPortAttachment error handling, file descriptor lifecycle, serial console tests, and the CI compile workflow timeout.

Verification

@Code-Hex
Code-Hex merged commit 7fd95b4 into main Sep 28, 2026
10 checks passed
@Code-Hex
Code-Hex deleted the codex/fix-serial-attachment-lifecycle branch September 28, 2026 11:32
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.

1 participant