appserver-io-php / fhreads

A PHP extension to get real system threading functionality in php userland.
Other
2 stars 1 forks source link

Unsetting variables in a threaded context leads to segfaults. #14

Open jensscherbl opened 8 years ago

jensscherbl commented 8 years ago

Especially when using array_shift(), array_pop() etc. in a thread for an array of anonymous functions.

jensscherbl commented 8 years ago

See https://github.com/jensscherbl/threading/blob/master/system/threading/class/QueueStore.php#L59 for an example.

The (memory leaking, debug only) workaround below produces random segfaults as well, but not always and right away.

https://gist.github.com/jensscherbl/8040d244ca8f4cfee436668c52d4355c

zelgerj commented 8 years ago

Ok, could you please provide an executable script to get it reproduced here. thx!

jensscherbl commented 8 years ago

Like this?

https://gist.github.com/jensscherbl/9e7b5c416fbd0a0f57c3491f9ad82807

zelgerj commented 8 years ago

perfect thanks, i'll take a look asap.

jensscherbl commented 8 years ago

Any progress so far?

zelgerj commented 8 years ago

Yes. The garbage collector destroys the list of callables $this->callables because the properties refcount is 1 in this moment. When the gc runs, it will fetch properties, every time so you have to keep care about refcounts on your own to control memory usage as well.

This should work:

<?php

final class Runnable
{
    private $callables;
    public function __construct(callable ...$callables)
    {
        $this->callables = $callables;
    }
    public function run()
    {
        // add ref count by assigning local function scope var to avoid gc
        $callables = $this->callables;

        do {
            $callable = array_shift($callables);
            $callable();
        } while (count($callables) > 0);
    }
}

$runnable = new Runnable(
    function () { echo 'Task 1.' . PHP_EOL; },
    function () { echo 'Task 2.' . PHP_EOL; },
    function () { echo 'Task 3.' . PHP_EOL; },
    function () { echo 'Task 4.' . PHP_EOL; },
    function () { echo 'Task 5.' . PHP_EOL; }
);

$id = null;
$handle = null;

fhread_create(
    $runnable,
    $id,
    $handle
);

fhread_join(
    $handle,
    $error
);

I also noticed that your script ends up with a seq-fault. I'll check this later.

Task 1.
Task 2.
Task 3.
Task 4.
Task 5.
[1]    37678 segmentation fault  sapi/cli/php test.php

You may want to have a look here where i've implemented your logic by using the fhreads lib Threadclass which does not end up with a seq-fault as an example: https://github.com/appserver-io/fhreads/blob/master/examples/Runnable.php

Cheers

jensscherbl commented 8 years ago

I also noticed that your script ends up with a seq-fault.

To be honest, I just assumed the segfault was a direct result of the gc issue.

i've implemented your logic by using the fhreads lib Threadclass which does not end up with a seq-fault

Kinda weird since the Thread class doesn't really do anything different under the hood. Could it be that this also has something to do with reference issues?

// add ref count by assigning local function scope var to avoid gc

How could this (temporary?) workaround be implemented for a simple Worker class?

https://gist.github.com/jensscherbl/fba47d61356e3f2e4e4bd9b39efa88c4

zelgerj commented 8 years ago

just temp. add

    public function run()
    {
        // add ref count by assigning local function scope var to avoid gc
        $callables = $this->callables;

to your workers run function.

cheers.

jensscherbl commented 8 years ago

just temp. add

Ah, got it. I somehow assumed it would create a thread-local copy instead of a reference. Not sure why...

zelgerj commented 8 years ago

ok. i'll leave this issue open until i was able to check if a fix on internals gc manipulation is possible. I'll keep it updated

jensscherbl commented 8 years ago

Let me know if there's anything I can do to help.