Skip to content

Initialize command state before thread creation - #198

Merged
holzschu merged 2 commits into
holzschu:masterfrom
demrom:fix/command-allocation-cleanup
Oct 23, 2026
Merged

holzschu merged 2 commits into
holzschu:masterfrom
demrom:fix/command-allocation-cleanup

Conversation

@demrom

@demrom demrom commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

When pthread_create fails, createOwnedCommand calls cleanup_function before run_function initializes the signal snapshot pointers. Cleanup frees each pointer and can abort (ios_system.m, the signal snapshot cleanup loop). Zero-initialize functionParameters before it reaches any cleanup path.

The arm64 Simulator regression uses Xcode 27.1 and a Release framework build. It forces one EAGAIN through an external thread-creation interposer with both MallocScribble and MallocPreScribble enabled. Master aborts with status 134; the fix returns 126, preserves the caller's streams and signal handler, and runs the next command with exact NEXT\n output.

An a-Shell build with this change passes normal command, file, pipeline, exit-status and background io.popen checks on an iPhone, with the same results as the unchanged build. The phone run does not inject a thread-creation failure.

@demrom
demrom marked this pull request as ready for review October 22, 2026 17:09
@holzschu

Copy link
Copy Markdown
Owner

I may be missing something obvious, but I'm not seeing what this PR changes in the ios_system.m code. I see just one change where malloc is replaced with calloc(1,...) and two changes in the comments.

@demrom

demrom commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

You're not missing anything: the fix is that one line. params came from malloc, so its signal snapshot pointers held whatever was in that memory. When pthread_create fails, cleanup_function runs before run_function has set them and frees each one, which aborts (status 134 in the test, with MallocScribble). With calloc they are NULL, so the cleanup is safe and the command returns 126. The two comments said "malloc'ed", which stopped being true. The rest of the PR is the regression test in tests/.

@holzschu

Copy link
Copy Markdown
Owner

Thanks for the explanation, and for catching that one.

@holzschu
holzschu merged commit b51dd90 into holzschu:master Oct 23, 2026
2 checks passed
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