Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GH-15905: Assertion failure for TRACK_VARS_SERVER #15930

Open
wants to merge 1 commit into
base: PHP-8.2
Choose a base branch
from

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Sep 16, 2024

When the superglobals are eagerly initialized, but "S" is not contained in variables_order, TRACK_VARS_SERVER is created as empty array with refcount > 1. Since this hash table may later be modified, a flag is set which allows such COW violations for assertions. However, when register_argc_argv is on, the so far uninitialized hash table is updated with argv, what causes the hash table to be initialized, what drops the allow-COW-violations flag. The following update with argc then triggers a refcount violation assertion.

Since we consider HT_ALLOW_COW_VIOLATION a hack, we do not want to keep the flag during hash table initialization, so we initialize the hash table right away after creation for this code path.

When the superglobals are eagerly initialized, but "S" is not contained
in `variables_order`, `TRACK_VARS_SERVER` is created as empty array
with refcount > 1.  Since this hash table may later be modified, a flag
is set which allows such COW violations for assertions.  However, when
`register_argc_argv` is on, the so far uninitialized hash table is
updated with `argv`, what causes the hash table to be initialized, what
drops the allow-COW-violations flag.  The following update with `argc`
then triggers a refcount violation assertion.

Since we consider `HT_ALLOW_COW_VIOLATION` a hack, we do not want to
keep the flag during hash table initialization, so we initialize the
hash table right away after creation for this code path.
@cmb69 cmb69 requested a review from bukka as a code owner September 16, 2024 23:19
@cmb69 cmb69 linked an issue Sep 16, 2024 that may be closed by this pull request
Copy link
Member

@arnaud-lb arnaud-lb left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

Assertion failure for TRACK_VARS_SERVER
2 participants