Skip to content

Fix use-after-free race on future->monitor in error paths#381

Merged
realFlowControl merged 1 commit into
developfrom
florian/fix-future-monitor-uaf-race
Jun 16, 2026
Merged

Fix use-after-free race on future->monitor in error paths#381
realFlowControl merged 1 commit into
developfrom
florian/fix-future-monitor-uaf-race

Conversation

@realFlowControl

Copy link
Copy Markdown
Collaborator

Root cause of the Windows worker-thread teardown access violation found via the diagnostic in #380.

The bug

When a task threw a normal exception (or crashed into the IllegalInstruction path), the worker signalled the future twice in php_parallel_scheduler_run():

  1. PHP_PARALLEL_ERROR (resp. READY|ERROR) when saving the task error, then
  2. an unconditional trailing php_parallel_monitor_set(future->monitor, PHP_PARALLEL_READY) at the end of the function.

Future::value() waits on READY|FAILURE|ERROR, so the first signal already releases the consumer. The consumer can then tear the future down — freeing future->monitor — before the worker reaches the second signal, which then dereferences a dangling monitor inside pthread_cond_signal → use-after-free.

On Windows this is a hard 0xC0000005 (caught here via a diagnostic backtrace):

php_parallel_thread            scheduler.c
 └ php_parallel_scheduler_run  -> monitor_set(future->monitor, PHP_PARALLEL_READY)
    └ php_parallel_monitor_set  monitor.c:93  pthread_cond_signal(&monitor->condition)
       └ <pthreads dll>  -> deref garbage (0x10000000A) -> AV

It surfaced in tests/functional/007.phpt (bootstrap set inside a task). On Linux the freed memory stayed readable, so it went unnoticed; on Windows it was masked by the catch-all VEH from #355.

The fix

The error paths now signal READY|ERROR exactly once and skip the trailing READY (guarded by a ready flag). Once that single monitor_set returns, the worker never touches the future again, closing the window. Setting READY in the error path also keeps the future destructor (which waits for READY) from blocking when value() is never called.

The success and kill paths are unchanged: they have no early wake and still rely on the trailing READY.

Testing

  • Linux suite green (148/148).
  • Windows CI on this PR should show functional/007 passing for the right reason (no AV).

Related

When a task threw (normal exception) or crashed (IllegalInstruction), the
worker signalled the future twice: first PHP_PARALLEL_ERROR (or READY|ERROR),
then an unconditional trailing PHP_PARALLEL_READY at the end of
php_parallel_scheduler_run().

Future::value() waits on READY|FAILURE|ERROR, so the first signal already
releases the consumer. The consumer can then tear the future down (freeing
future->monitor) before the worker runs the trailing
php_parallel_monitor_set(future->monitor, PHP_PARALLEL_READY), which then
dereferences a dangling monitor inside pthread_cond_signal -> access
violation / use-after-free.

This was observed as a hard crash on Windows (0xC0000005) in
tests/functional/007.phpt, masked until now by the catch-all VEH; on Linux
the freed memory happened to stay readable.

Fix: the error paths now signal READY|ERROR exactly once and skip the
trailing READY (via a 'ready' flag). The success and kill paths are
unchanged - they have no early wake and still rely on the trailing READY.
Setting READY in the error path also ensures the future destructor (which
waits for READY) never blocks when value() is not called.
@realFlowControl
realFlowControl marked this pull request as ready for review June 16, 2026 11:01
@realFlowControl
realFlowControl merged commit d0dee71 into develop Jun 16, 2026
72 of 88 checks passed
@realFlowControl
realFlowControl deleted the florian/fix-future-monitor-uaf-race branch June 16, 2026 11:41
@realFlowControl realFlowControl added this to the v1.2.15 milestone Jun 18, 2026
@realFlowControl realFlowControl changed the title Fix use-after-free race on future->monitor in error paths Fix use-after-free race on future->monitor in error paths Jun 22, 2026
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