Promise timer wont stop sub-processes

[Parsoolak]

Hi dear @clue ,
I Think I found a bug!
or I am just confused how this promise-timer works!

When I below Code, My Base Promise Canceled But Sub-processes wont stop and continue their works

<?php

use React\Promise\Promise;
use React\Promise\Timer\TimeoutException;
use function React\Promise\Timer\timeout;

require "vendor/autoload.php";

// Setup EventLoop
$loop = React\EventLoop\Loop::get();

timeout(
    new Promise(function () {
        echo "Task Start" . PHP_EOL;
        return \React\Promise\Timer\sleep(10)
            ->then(function () {
                echo "Sleep Complete and Run" . PHP_EOL;
            });
    }, function () {
        echo "Promise Canceled" . PHP_EOL;
    }), 5)
    ->then(function ($result) {
        // on Success
        echo "Task Success Finalized";
        return true;
    }, function (\Exception $error) {
        // on Timeout/failed
        if ($error instanceof TimeoutException) {
            echo "Task Timeout-ed" . PHP_EOL;
        } else {
            echo "Task Exception" . PHP_EOL;
        }

        return false;
    });


$loop->run();

and Code Response:

Task Start
Task Timeout-ed
Promise Canceled
Sleep Complete and Run

Why it act like that and wont stop sub-processes?
whats wrong in my Ideology؟

[hasanparasteh]

You have to resolve the promise you are passing into timeout function. if you do not resolve it using the callables provided with Promise it won't work as expected! here it is the patched version of your code:

new Promise(function (callable $resolve) {
    echo "Task Start" . PHP_EOL;
    $resolve(
        \React\Promise\Timer\sleep(10)
            ->then(function () {
                echo "Sleep Complete and Run" . PHP_EOL;
            })
    );
});

and the output should be like below if you use promises correctly:

Task Start
Task Timeout-ed

[Parsoolak]

Got it @hasanparasteh ! Thanks

[Parsoolak]

I had some workarounds and the issue still exist although I'm using resolver to resolve the promise. I found out that the React\Promise\Timer\timeout won't timeout promises recursively. I mean if you have a promise which resolves a sleep and then again after some time another sleep occurs the second sleep hanging int the background and it won't cancel as expected... This issue takes unnecessary amount of CPU, RAM and etc. Also it may occur some logical issues..

here is an example:

timeout(new Promise(function (callable $resolve) {
    echo "Task Start" . PHP_EOL;

    $resolve(\React\Promise\Timer\sleep(4)
        ->then(function () {
            echo "First Sleep Run" . PHP_EOL;

            // It Must Be Canceled Here But it run
            return \React\Promise\Timer\sleep(5)
                ->then(function () {
                    echo "Second Sleep Run" . PHP_EOL;
                });
        }));
}), 5)->then(function ($result) {
    echo "Task Resolved Success" . PHP_EOL;
}, function (\Exception $exception) {
    echo "Task Timeout" . PHP_EOL;
});

[WyriHaximus]

Looks like you're locking a canceler to pass on the timeout canceling the initial promise:

$p = null;
timeout(new Promise(function (callable $resolve) use (&$p) {
    echo "Task Start" . PHP_EOL;

    $resolve($p = \React\Promise\Timer\sleep(4)
        ->then(function () {
            echo "First Sleep Run" . PHP_EOL;

            // It Must Be Canceled Here But it run
            return \React\Promise\Timer\sleep(5)
                ->then(function () {
                    echo "Second Sleep Run" . PHP_EOL;
                });
        }));
}, function () use (&$p) {
    $p->cancel();
}, 5)->then(function ($result) {
    echo "Task Resolved Success" . PHP_EOL;
}, function (\Exception $exception) {
    echo "Task Timeout" . PHP_EOL;
});

[Parsoolak]

It seems like your solution won't work either. the inner promise keeps working and it won't cancel in promise using it's reference.

[WyriHaximus]

It seems like your solution won't work either. the inner promise keeps working and it won't cancel in promise using it's reference.

It should cancel it, wrote that code without running it because that should be enough. Having a deeper dive

[SimonFrings]

@Parsoolak Can you tell us a bit more what you're trying to achieve, understanding your use case helps us to guide you in the right direction.

It's hard to make out what's actually happening in your example, did you take a look at the promise documentation?

[WyriHaximus]

@SimonFrings Technically @Parsoolak has a very good point as that I would also expect that to work. (This one specifically #65 (comment) .) However when using that $resolve there is no way to keep track of that cancelation in that specific situation. I have some tryouts somewhere that I'd also expected to work but didn't. Will get back to that tomorrow because this should be not that hard to resolve.

[SimonFrings]

@WyriHaximus Maybe it makes more sense to try another approach to reach a result, instead of trying to fix the code example above. Promises, especially with cancellation, can be tricky from time to time and is not perfect, there's also been quite a discussion about this in reactphp/promise#56.

I'd suggest to try out https://github.com/reactphp/async, using the async() and await() functions might be easier in this case, as this will also be the way forward for ReactPHP (v3 and v4).

All in all it seems to me this ticket isn't about a problem in https://github.com/reactphp/promise-timer, but more suited for https://github.com/reactphp/promise.

What do you guys think?

[Parsoolak]

I agree. lets move over https://github.com/reactphp/promise and talk more about this issue.