Skip to content

CoW involved sometimes /w opcache #18600

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

Closed
mvorisek opened this issue May 19, 2025 · 7 comments
Closed

CoW involved sometimes /w opcache #18600

mvorisek opened this issue May 19, 2025 · 7 comments

Comments

@mvorisek
Copy link
Contributor

mvorisek commented May 19, 2025

Description

The following code:

minimal repro repo: https://github.com/atk4/core/tree/debug_ref_change_with_opcache_and_coverage

It contains 2 files.

  • .github/workflows/test-unit.yml - working CI on push
  • repro.php

Resulted in this output:

array(2) {
  [0] =>
  string(40) "4e76db9f1723ab3273cbb0453e6a3886c7ffc04a"
  [1] =>
  string(40) "ef428dd20aa28afe07ce1a36ff921fcfc9fa5baf"
}

But I expected this output instead:

no output - references are the same

My observations when minimalizing the reproducer:

The repro contains simple repro.php reproduced.

When run using the CI, it fails with the unexpected result under these conditions:

  1. xdebug extension is installed
  2. opcache is enabled for CLI

The CoW, ie. references updated, is involved in most runs, but not always. In the CI, I run the repro 10x and with >90% probablity at least one of PHP 8.2/8.3/8.4 job fails.

I am not sure if xdebug needs to be installed, but it helps to reproduce the issue more frequently.

Opcache seems however always needed.

Disabled GC has no impact.

Also it seems the Closure::bind in the repro is needed to reproduce.

Please confirm you can reproduce this issue on your side.

PHP Version

PHP 8.2.28 (cli) (built: May 15 2025 03:24:27) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.2.28, Copyright (c) Zend Technologies
    with Zend OPcache v8.2.28, Copyright (c), by Zend Technologies
    with Xdebug v3.4.3, Copyright (c) 2002-2025, by Derick Rethans

Operating System

Ubuntu

@iluuu1994
Copy link
Member

iluuu1994 commented May 19, 2025

I don't understand what is being tested here. $makeCaseFx creates a reference but that reference is destroyed again once the function ends (return [$v, [ does not persist the reference, you would need return [&$v, [). Most likely, you're just getting the same hash because the allocator ends up re-using the freed up slot. Can you clarify where exactly you think a problem occurs?

@mvorisek
Copy link
Contributor Author

mvorisek commented May 19, 2025

The code is used to obtain "id of a variable" to be later used to prevent recursion when dumping an array. The code in the repro is minimal code needed to reproduce it without much meaning.

I know there is no guarantee from the language that reference id will not change once there is no reference active.

But so far I understand the design the reference id is computed as a hash of zval ptr.

This is shown in https://3v4l.org/710ps.

So the issue reported here is what opcache does to the variable so that the zval is copied and if that can be avoided. Maybe something can be improved.

@iluuu1994
Copy link
Member

iluuu1994 commented May 19, 2025

I know there is no guarantee from the language that reference id will not change once there is no reference active.

The reference is not just "inactive", the reference is gone, it is freed. In your example, the new reference by chance just happens to live in the same memory where the previous reference lived. Essentially, the buffered getId() is a dangling pointer, and you should not rely on it. See this example, once there is something else occupying the previously freed memory, you'll get a new memory location. https://3v4l.org/u8tSK

@iluuu1994 iluuu1994 closed this as not planned Won't fix, can't repro, duplicate, stale May 19, 2025
@mvorisek
Copy link
Contributor Author

Why the repro is fixed by atk4/core@12e4abc this change?

@iluuu1994
Copy link
Member

@mvorisek It's hard to tell when you're relying on memory allocations being placed in specific slots. The point remains: If your allocation is freed, the value returned from getId() essentially becomes invalid/stale. You cannot rely on it after, it may be reused for some completely unrelated allocation at a later point.

@mvorisek
Copy link
Contributor Author

mvorisek commented May 19, 2025

Here is the usecase I need to solve:

<?php

$arr = [4];
$o = new stdClass();
$o->foo = &$arr[0];

$arr[0] = 10;

print_r($o);

// given $o, tell me which properties are referenced to $arr

repro: https://3v4l.org/6KKQu

Currently, I use:

function getRid(&$value): string
{
    return \ReflectionReference::fromArrayElement([&$value], 0)->getId();
}

to get a variable reference id.

I understand it can return the same result if variable, more precisely zval, is released and allocated again.

This implies me these questions:

How can I get a reference id from a any variable or an object property?

Currently the only function that can return reference id I am aware of is the ReflectionReference::fromArrayElement which is for array values only.

Shouldn't there be also a fromProperty($o, $k, $scope) method?

If I use the getRid solution, I cannot validate if the argument was a reference before or not. If I cannot validate it, I cannot trust the result per your words.

However.

What is a reference internally and what is the reference id

If I am right, the reference id (in sense of ReflectionReference::getId() output) is a hashed ptr of a zval.

A zval is allocated on the first assign.

The situation when zval is copied is when it is to be modified and has refcount>1. Changing variable to be a reference, or a reference passed by a value implies modification.

When the refcount=1, no zval copy is done as not needed.

So if this is true, the getRid solution is perfectly fine, because the zval is kept alive no matter if it is a reference during the passing or not. As discussed above, this is true only when refcount=1, which is true in the repro.

So in opcache (or possibly xdebug), there is something that increases refcount of the passed variable causing the zval to be split (copied). And hence this issue as refcount should not be increased with opcache more than without, as it has side effects.

@iluuu1994
Copy link
Member

How can I get a reference id from a any variable or an object property?

You can't get the id for any variable. You can get it for properties like this: https://3v4l.org/DXVnX

There seem to be some misunderstandings in your explanation. Zvals are not separate from references. Zvals can either live on the stack or in the zend_reference allocation. getId() always refers to the zend_reference pointer, stack zvals can never be referenced. Nor can array elements, they are volatile and may be moved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants