parallel icon indicating copy to clipboard operation
parallel copied to clipboard

zend_mm_heap corrupted

Open mvorisek opened this issue 1 year ago • 7 comments

repro branch: https://github.com/php-lock/lock/tree/test_37

steps to reproduce:

  1. clone the repo
  2. run composer update
  3. run php vendor/bin/phpunit --no-coverage
Runtime:       PHP 8.3.14
Configuration: /__w/lock/lock/phpunit.xml.dist

...............................................................  63 / 169 ( 37%)
...............1 Running 
zend_mm_heap corrupted
2 Running 
3 Running 
Aborted (core dumped)

mvorisek avatar Dec 15 '24 00:12 mvorisek

On PHP 7.4 and 8.0 I get:

1) Malkusch\Lock\Tests\Mutex\FlockMutexTest::testThreadLock
Error: Using $this when not in object context

/__w/lock/lock/tests/Mutex/FlockMutexTest.php:123

which points to https://github.com/php-lock/lock/blob/7075016e8b44fe7d2e61bab913d56d5ca79fec51/tests/Mutex/FlockMutexTest.php#L123.

It seems there is some refcounting issue.

mvorisek avatar Dec 15 '24 14:12 mvorisek

Hey @mvorisek 👋

thanks for raising this issue! I might have limited time over the next few weeks, so do not expect any updates on this.

There are some things you could do to help me narrow the problem down:

  • try running with different PHP versions (8.0 to 8.4)
  • try with and without OPcache (make sure OPcache on CLI is enabled)
  • try with older versions of parallel (v1.2.4 vs. v1.2.5)
  • provide a backtrace without ZendMM:
    $ USE_ZEND_ALLOC=0 gdb --args php vendor/bin/phpunit --no-coverage
    
    Once it fails, type bt to get a backtrace and post that here.

This might help a bit, but I am not sure. I might need to run this with ASAN to get to the root cause.

realFlowControl avatar Dec 17 '24 07:12 realFlowControl

Not sure if a backtrace would help here, as the variable used in the Closure is released too early, it seems the Closure passed to \parallel\run somehow does not hold the references until all parallel tasks are executed.

mvorisek avatar Dec 17 '24 12:12 mvorisek

Hey @mvorisek,

sorry it took a while, but I found some segfaults and heap corruption and fixed them in https://github.com/krakjoe/parallel/pull/339, can you try the version from that branch and see if that fixes your problem?

realFlowControl avatar Mar 11 '25 20:03 realFlowControl

Thank you very much!

see if that fixes your problem?

I do not have more failing test cases then the one posted here, so if it fixed, then 🟢 light from my side.

mvorisek avatar Mar 11 '25 21:03 mvorisek

Ah sorry, I was too fast, it still falis

realFlowControl avatar Mar 12 '25 07:03 realFlowControl

I think I see what is going on, the test failing is the FlockMutextTest::testThreadLock():

    public function testThreadLock(): void
    {
        $syncFunction = function ($i) {
            try {
                // require_once __DIR__ . '/../../../bootstrap/app.php';
                echo $i . ' Running ' . \PHP_EOL;

                return $this->mutex->synchronized(function () use ($i): string {
                    sleep(1);

                    return $i . ' Finished.' . \PHP_EOL;
                });
            } catch (\Exception $e) {
                return $e->getMessage() . \PHP_EOL;
            }
        };

        $results = [];
        for ($i = 1; $i < 6; ++$i) {
            $results[] = \parallel\run($syncFunction, [$i]);
        }

        foreach ($results as $item) {
            echo $item->value();
        }
    }
}

Run with:

php vendor/bin/phpunit --no-coverage --filter FlockMutexTest::testThreadLock

The offending line is:

return $this->mutex->synchronized(function () use ($i): string {

You would assume that $this refers to your FlockMutexTest-object , but using it in a thread means that the object you refer to is from another thread. The access to $this is not synchronised and actually not what you'd expect it to be. Once the second thread is accessing the $this variable, it is an empty object with an invalid internal state.

How to proceed

Whatever you pass into a closure to \parallel\run() or \parallel\Runtime::run() needs to be self contained. You are only allowed to pass in data using the second argument to those functions or a use statement. See the Task- and Argument Characteristics sections in the docs for what is allowed to pass in. I already checked, but the $this->mutex object does not obey to the argument characteristics and as such will result in a this error:

parallel\Runtime\Error\IllegalParameter: illegal parameter (Eloquent\Liberator\LiberatorObject) passed to task at argument 2

It might help you to know, that you can achieve thread synchronisation with a \parallel\Sync-object, see usage at:

https://github.com/krakjoe/parallel/blob/c40c2c181ff57a321015f91c50c66e95c59ea616/tests/sync/002.phpt

And find the docs at https://www.php.net/manual/de/class.parallel-sync.php

Also adding that the https://github.com/eloquent/liberator library has been archived ~1.5 years ago.

realFlowControl avatar Mar 12 '25 08:03 realFlowControl