__get/__set + coroutines workaround?

[ValiDrv]

1. What did you do? If possible, provide a simple script for reproducing the error.

We work with allot of data transfer objects which can be chained like this: $obj->child->child->property; which are lazy loaded from the database on access.

The problem is, if we use a coroutine to wait for the database results when we access ->child, the second request on the same object will fail with a Notice: Undefined property:

It seems to work using __call magic method, $obj->child()->child()->property();

Is there a way to make __get/__set work/is there a workaround to make $obj->child->child->property; work property when dealing with coroutines?

Test code

<?php declare(strict_types=1);
error_reporting(-1);

class  DTO2 {
    public string $data = '';

    public function __construct(string $data) {
        \Swoole\Coroutine::sleep(0.1);  # Simulate waiting for DB
        $this->data = $data;
    }

    public function __toString() {
        return $this->data;
    }
}

/**
 * @property string $record Database record
 */
class  DTO1 {
    private array $dbData = [];

    public function &__get(string $key) {
        return $this->_get($key);
    }

    public function &_get(string $key) {
        $this->dbData[$key] ??= new DTO2($key); # Lazy load the data from the DB

        return $this->dbData[$key];
    }

    public function &__call($name, $arguments) {
        return $this->_get($name);
    }
}

$t = new  DTO1();
for ($i = 0; $i < 5; $i++) {
    go(function () use ($t, $i) {
        echo $t->record . "\n";     # Works only once
        echo $t->record() . "\n";     # Works
        //        echo $t->_get('record') . "\n"; # Works
    });
}

Run the code:

docker run -v $(pwd)/test.php:/test.php --rm phpswoole/swoole  "php /test.php 2>&1"

2. What did you expect to see?

The __get function to be called 5 times.

record
record
record
record
record

3. What did you see instead?

The __get function got called only once, and 4 times it seems the __get does not exist.

Notice: Undefined property: DTO1::$record in /test1.php on line 41
Notice: Undefined property: DTO1::$record in /test1.php on line 41
Notice: Undefined property: DTO1::$record in /test1.php on line 41
Notice: Undefined property: DTO1::$record in /test1.php on line 41
record

**4. What version of Swoole are you using (show your php --ri swoole)?

5. What is your machine environment used (show your uname -a & php -v & gcc -v) ?**

phpswoole/swoole

Ps: Google translate for: https://wiki.swoole.com/#/coroutine/notice?id=%e5%9c%a8-__get-__set-%e9%ad%94%e6%9c%af%e6%96%b9%e6%b3%95%e9%87%8c%e4%b8%8d%e5%be%97%e4%ba%a7%e7%94%9f%e5%8d%8f%e7%a8%8b%e5%88%87%e6%8d%a2 Suggests to implement get/ set call method explicitly.

How can that be done without: $obj->_get('child')->_get('child')->_get('property'); ?

[matyhtf]

The ZendVM has limitations, cannot suspend coroutine in the get/set magic methods.

When using __get for the first time, it will mark ZEND_ACC_USE_GUARDS on the class entity, The second time it will directly return null.

There is no solution, this needs to modify zendvm

[twose]

Duplicate with https://github.com/swoole/swoole-src/issues/2625

[ValiDrv]

@matyhtf thank you, that makes sense.

I do suggest tho, to make Swoole throw a [Swoole]RuntimeException if ever a coroutine tries to be suspended inside a get/set, if possible.

[matyhtf]

@ValiDrv Swoole cannot determine whether it is in the get/set method. When switching the coroutine, the scheduler needs to analyze all call stacks, and the performance loss is large

[ValiDrv]

@matyhtf I totally understand that, but I don't think you need to track anything actually, and the performance hit would only happen when an exception is thrown, where the code logic would break anyway.

I'm not familiar with the Swoole C code, but I assume there is some kind of global error handler wrapping the entire code.

The PHP equivalent would be something like this:

try {
    # Swoole code
}catch (Throwable $e) {
    # Global error handler

    # Some smarter future proof way to
    # - match only "Notice: Undefined property: DTO1::$record in /test1.php on line 41" (Maybe by PHP internal error code)
    # - match a list of php calls which use ZEND_ACC_USE_GUARDS, in this case ->__get() & ->__set()
    # - The performance hit (loop getTraceAsString) should only happen when an exception is thrown (code could not work either way here)
    if (preg_match('~Undefined property:~i', $e->getMessage()) && preg_match('~->(?:__get|__set)\(\)$~m', $e->getTraceAsString())) {
        # Reword the exception
        throw new RuntimeException("Cannot suspend coroutine in the __get/__set magic methods.");
    }
    # Send original
    throw $e;
}

[twose]

@ValiDrv

Yes, we can do that, but not good, you even use regex...

This is actually the same as multi-threaded access to the same resource, the OS will not give you any warning/exception. Of course, they are not the same, I think you can implement the lock mechanism through channels, but it is not very convenient to write.

The best way is not to use magic methods

[ValiDrv]

Thanks. The regex was just a pseudo code example.

We wrapped all coroutine calls in a custom function, which in dev can check the call stack and throw an error of its called from get/set. (it's not ideal).

Doing so, we found a few places coroutines were called from __get, which made logical sense in the grand scheme of things.