Skip to content

Fix GH-22567 (Windows ZTS CLI SAPI should refresh its TSRMLS cache during request activation) - #22568

Closed
matyhtf wants to merge 1 commit into
php:masterfrom
matyhtf:fix-cli-ts-crash-on-windows
Closed

matyhtf wants to merge 1 commit into
php:masterfrom
matyhtf:fix-cli-ts-crash-on-windows

Conversation

@matyhtf

@matyhtf matyhtf commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread sapi/cli/php_cli.c
Comment on lines +355 to +362
static int sapi_cli_activate(void) /* {{{ */
{
#if defined(PHP_WIN32) && defined(ZTS)
ZEND_TSRMLS_CACHE_UPDATE();
#endif
return SUCCESS;
}
/* }}} */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
static int sapi_cli_activate(void) /* {{{ */
{
#if defined(PHP_WIN32) && defined(ZTS)
ZEND_TSRMLS_CACHE_UPDATE();
#endif
return SUCCESS;
}
/* }}} */
static int sapi_cli_activate(void)
{
#if defined(PHP_WIN32) && defined(ZTS)
ZEND_TSRMLS_CACHE_UPDATE();
#endif
return SUCCESS;
}

Those brackets are not necessary anymore

@Girgias Girgias changed the title Fix GH-22567 Fix GH-22567 (Windows ZTS CLI SAPI should refresh its TSRMLS cache during request activation) Sep 6, 2026
@Girgias

Girgias commented Sep 6, 2026

Copy link
Copy Markdown
Member

@henderkes as you've been looking at ZTS recently care to have a look?

@henderkes henderkes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Old brace thingies aside, this is correct. I assume swoole runs into it in its threaded worker handler mode.

@Girgias

Girgias commented Sep 6, 2026

Copy link
Copy Markdown
Member

Old brace thingies aside, this is correct. I assume swoole runs into it in its threaded worker handler mode.

I guess this needs bacporting to 8.4 too?

@Girgias Girgias closed this in 4e64a26 Sep 19, 2026
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.

4 participants