Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions UPGRADING.INTERNALS
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,11 @@ PHP 8.4 INTERNALS UPGRADE NOTES
4. OpCode changes
========================

* DO_ICALL, DO_FCALL, and DO_FCALL_BY_NAME now call zend_interrupt_function
while the internal frame is still on the stack. This means interrupt handlers
will now see the internal call. If your interrupt handler does something like
switching EG(current_execute_data), it should not do so if an internal func
is on top.
* New FRAMELESS_ICALL_[0,3] opcodes for faster internal function calls have been
added. These opcodes don't create a stack frame, but pass arguments via opcode
operands. They only work for functions that are known at compile-time, and
Expand Down
11 changes: 6 additions & 5 deletions Zend/tests/fibers/signal-async.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pcntl_signal(SIGUSR1, function (): void {
$fiber = new Fiber(function (): void {
echo "Fiber start\n";
posix_kill(posix_getpid(), SIGUSR1);
time_nanosleep(1);
time_nanosleep(1, 0);
echo "Fiber end\n";
});

Expand All @@ -30,8 +30,9 @@ Fiber start
Fatal error: Uncaught FiberError: Cannot switch fibers in current execution context in %ssignal-async.php:%d
Stack trace:
#0 %ssignal-async.php(%d): Fiber::suspend()
#1 %ssignal-async.php(%d): {closure:%s:%d}(%d, Array)
#2 [internal function]: {closure:%s:%d}()
#3 %ssignal-async.php(%d): Fiber->start()
#4 {main}
#1 [internal function]: {closure:%s:%d}(%d, Array)
#2 %ssignal-async.php(%d): posix_kill(%d, %d)
#3 [internal function]: {closure:%s:%d}()
#4 %ssignal-async.php(%d): Fiber->start()
#5 {main}
thrown in %ssignal-async.php on line %d
15 changes: 15 additions & 0 deletions Zend/zend.h
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,22 @@ extern ZEND_API size_t (*zend_printf)(const char *format, ...) ZEND_ATTRIBUTE_PT
extern ZEND_API zend_write_func_t zend_write;
extern ZEND_API FILE *(*zend_fopen)(zend_string *filename, zend_string **opened_path);
extern ZEND_API void (*zend_ticks_function)(int ticks);

/* Called by the VM in certain places like at the loop header, user function
* entry, and after internal function calls, if EG(vm_interrupt) has been set.
*
* If this is used to switch the EG(current_execute_data), such as implementing
* a coroutine scheduler, then it needs to check the top frame to see if it's
* an internal function. If an internal function is on top, then the frame
* shouldn't be switched away.
*
* Prior to PHP 8.0, this check was not necessary. In PHP 8.0,
* zend_call_function started calling zend_interrupt_function, and in 8.4 the
* DO_*CALL* opcodes started calling the zend_interrupt_function while the
* internal frame is still on top.
*/
extern ZEND_API void (*zend_interrupt_function)(zend_execute_data *execute_data);

extern ZEND_API void (*zend_error_cb)(int type, zend_string *error_filename, const uint32_t error_lineno, zend_string *message);
extern ZEND_API void (*zend_on_timeout)(int seconds);
extern ZEND_API zend_result (*zend_stream_open_function)(zend_file_handle *handle);
Expand Down
9 changes: 9 additions & 0 deletions Zend/zend_execute.c
Original file line number Diff line number Diff line change
Expand Up @@ -4056,6 +4056,15 @@ ZEND_API void ZEND_FASTCALL zend_free_compiled_variables(zend_execute_data *exec
}
/* }}} */

ZEND_API ZEND_COLD void ZEND_FASTCALL zend_interrupt_or_timeout(zend_execute_data *call)
{
if (zend_atomic_bool_load_ex(&EG(timed_out))) {
zend_timeout();
} else if (zend_interrupt_function) {
zend_interrupt_function(call);
}
}

#define ZEND_VM_INTERRUPT_CHECK() do { \
if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { \
ZEND_VM_INTERRUPT(); \
Expand Down
12 changes: 12 additions & 0 deletions Zend/zend_execute.h
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,18 @@ ZEND_COLD void zend_magic_get_property_type_inconsistency_error(const zend_prope

ZEND_COLD void zend_match_unhandled_error(const zval *value);

/* Call this to handle the timeout or the interrupt function. It will not clear
* the EG(vm_interrupt).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you rely on zend_interrupt_function to clear vm_interrupt in this case? Should we still reset vm_interrupt in case of timeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed up a commit while you were reviewing ^_^
The store is done in the VM with atomic exchange which is better than a load + store individually. However in JIT, I don't have an IR instruction for atomic exchange, so there it does a load + store.

*/
ZEND_API ZEND_COLD void ZEND_FASTCALL zend_interrupt_or_timeout(zend_execute_data *call);

static zend_always_inline void zend_interrupt_or_timeout_check(zend_execute_data *call)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This function is similar to ZEND_VM_INTERRUPT_CHECK() and ZEND_VM_LOOP_INTERRUPT_CHECK(), but with subtle differences.

I think it would help understanding if this function was explicitly implemented as a variant of these two macros. E.g. it could be implemented as a macro named ZEND_VM_FCALL_INTERRUPT_CHECK().

{
if (UNEXPECTED(zend_atomic_bool_exchange_ex(&EG(vm_interrupt), false))) {
zend_interrupt_or_timeout(call);
}
}

static zend_always_inline void *zend_get_bad_ptr(void)
{
ZEND_UNREACHABLE();
Expand Down
12 changes: 9 additions & 3 deletions Zend/zend_vm_def.h
Original file line number Diff line number Diff line change
Expand Up @@ -4074,6 +4074,7 @@ ZEND_VM_HOT_HANDLER(129, ZEND_DO_ICALL, ANY, ANY, SPEC(RETVAL,OBSERVER))
}
#endif
ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret);
zend_interrupt_or_timeout_check(call);

EG(current_execute_data) = execute_data;
zend_vm_stack_free_args(call);
Expand All @@ -4097,7 +4098,8 @@ ZEND_VM_HOT_HANDLER(129, ZEND_DO_ICALL, ANY, ANY, SPEC(RETVAL,OBSERVER))
HANDLE_EXCEPTION();
}

ZEND_VM_SET_OPCODE(opline + 1);
CHECK_SYMBOL_TABLES()
OPLINE = opline + 1;
ZEND_VM_CONTINUE();
}

Expand Down Expand Up @@ -4195,6 +4197,7 @@ ZEND_VM_HOT_HANDLER(131, ZEND_DO_FCALL_BY_NAME, ANY, ANY, SPEC(RETVAL,OBSERVER))
}
#endif
ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret);
zend_interrupt_or_timeout_check(call);

EG(current_execute_data) = execute_data;

Expand Down Expand Up @@ -4225,7 +4228,8 @@ ZEND_VM_C_LABEL(fcall_by_name_end):
zend_rethrow_exception(execute_data);
HANDLE_EXCEPTION();
}
ZEND_VM_SET_OPCODE(opline + 1);
CHECK_SYMBOL_TABLES()
OPLINE = opline + 1;
ZEND_VM_CONTINUE();
}

Expand Down Expand Up @@ -4315,6 +4319,7 @@ ZEND_VM_HOT_HANDLER(60, ZEND_DO_FCALL, ANY, ANY, SPEC(RETVAL,OBSERVER))
}
#endif
ZEND_OBSERVER_FCALL_END(call, EG(exception) ? NULL : ret);
zend_interrupt_or_timeout_check(call);

EG(current_execute_data) = execute_data;

Expand Down Expand Up @@ -4344,7 +4349,8 @@ ZEND_VM_C_LABEL(fcall_end):
HANDLE_EXCEPTION();
}

ZEND_VM_SET_OPCODE(opline + 1);
CHECK_SYMBOL_TABLES()
OPLINE = opline + 1;
ZEND_VM_CONTINUE();
}

Expand Down
42 changes: 33 additions & 9 deletions Zend/zend_vm_execute.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions ext/opcache/jit/zend_jit.c
Original file line number Diff line number Diff line change
Expand Up @@ -1425,9 +1425,7 @@ static int zend_jit(const zend_op_array *op_array, zend_ssa *ssa, const zend_op
zend_jit_set_last_valid_opline(&ctx, op_array->opcodes + ssa->cfg.blocks[b].start);
}
if (ssa->cfg.blocks[b].flags & ZEND_BB_LOOP_HEADER) {
if (!zend_jit_check_timeout(&ctx, op_array->opcodes + ssa->cfg.blocks[b].start, NULL)) {
goto jit_failure;
}
zend_jit_check_timeout(&ctx, op_array->opcodes + ssa->cfg.blocks[b].start, NULL);
}
if (!ssa->cfg.blocks[b].len) {
zend_jit_bb_end(&ctx, b);
Expand Down
Loading