Skip to content

Conversation

Appla
Copy link
Contributor

@Appla Appla commented Sep 10, 2025

When zend-max-execution-timers is enabled, this test case consistently fails.
This patch aligns its behavior with other types of timeout implementations.

I'm unsure whether related code paths—such as main.c#L2523 and php_cli_server.c#L2278—should also be updated.

@iluuu1994
Copy link
Member

iluuu1994 commented Sep 10, 2025

From what I understand, this happens because zend_timeout_handler() installs the EG(hard_timeout) handler, but zend_interrupt_helper() immediately calls zend_timeout(), which calls zend_set_timeout_ex(0, 1);. This clears the installed hard timeout and makes the shutdown procedure loop forever.

I wonder whether we should call zend_set_timeout_ex(0, 1); in zend_timeout() at all? Maybe you know, @arnaud-lb? Removing it also resolves the issue, though your patch likely also makes sense to unify the different timers.

Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

Looks reasonable as-is, awaiting Arnaud's answer for my question. 🙂

@arnaud-lb
Copy link
Member

@iluuu1994 I'm not sure, but I think that calling zend_set_timeout_ex(0, 1) is necessary because the signal handler is set with SA_RESETHAND in some cases, so it needs to be reinstalled after it fires.

The proposed change looks right to me, and is consistent with other timeout implementations.

@iluuu1994 iluuu1994 closed this in ed9430a Sep 11, 2025
@iluuu1994
Copy link
Member

Thank you @Appla!

iluuu1994 added a commit that referenced this pull request Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants