advitrocks9 opened a new pull request, #51107: URL: https://github.com/apache/arrow/pull/51107
### Rationale for this change `LaunchWorkersUnlocked` appends to `state_->workers_` before constructing the thread that owns that entry, and only the worker itself erases it. If the `std::thread` constructor throws, the entry stays, so `Shutdown` waits forever on `workers_.empty()`. The same throw escapes `SpawnReal` after `tasks_queued_or_running_` has been incremented, which wedges `WaitForIdle`, and once stale entries fill `workers_` to capacity the pool stops launching workers while `Spawn` still returns OK for tasks nothing will run. ### What changes are included in this PR? The thread construction is wrapped so a failure erases the entry and rethrows, and the counter is incremented only after the launch succeeds. `Spawn` already propagates `std::system_error` from this path. Turning it into a `Status` would report success for a task with no worker. ### Are these changes tested? `TestThreadPoolForkSafety.FailedWorkerLaunch` forces a thread creation failure in a forked child, then checks the pool is back to zero workers and zero tasks and still shuts down. It fails on main, and reverting either hunk brings the failure back. The child skips if it cannot make thread creation fail. ### Are there any user-facing changes? No. A failed spawn throws the same `std::system_error` and the pool stays usable. * GitHub Issue: #48137 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
