openswoole / ext-openswoole

Programmatic server for PHP with async IO, coroutines and fibers
https://openswoole.com
Apache License 2.0
808 stars 51 forks source link

[bug] usleep function (with hooks) - unexpected behaviour (without actually delay) #236

Closed mrVrAlex closed 2 years ago

mrVrAlex commented 2 years ago
  1. What did you do? If possible, provide a simple script for reproducing the error.
    Swoole\Runtime::enableCoroutine(true);
    Co\run(function () {
    go(function () {
        $t = microtime(true);
        usleep(10000);
        echo (microtime(true) - $t) * 1e6 . PHP_EOL;
    });
    });
  2. What did you expect to see?

something like value with 10k microseconds. So I expected SAME result if I run Co::usleep(10000);

Swoole\Runtime::enableCoroutine(true);
Co\run(function () {
    go(function () {
        $t = microtime(true);
        Co::usleep(10000);
        echo (microtime(true) - $t) * 1e6 . PHP_EOL;
    });
});
  1. What did you see instead?

First code not return same result as second code. All values less then 1 000 000 works WITHOUT delay by fact with usleep() function.

Co::usleep() works as expected.

  1. What version of OpenSwoole are you using (show your php --ri openswoole)?
    
    4.11.1
6. What is your machine environment used (show your `uname -a` & `php -v` & `gcc -v`) ?

Linux platform-worker-75ccf956d4-c4frs 5.10.109+ #1 SMP Sun Apr 24 09:09:13 UTC 2022 x86_64 GNU/Linux

PHP 8.1.8 (cli) (built: Jul 7 2022 22:48:08) (NTS) Copyright (c) The PHP Group Zend Engine v4.1.8, Copyright (c) Zend Technologies with Zend OPcache v8.1.8, Copyright (c), by Zend Technologies

doubaokun commented 2 years ago

This is by design, due to the execution cost and preciseness. usleep less than 1ms is ignored by the engine.

mrVrAlex commented 2 years ago

@doubaokun I do not understand - you say about "usleep less than 1ms" - in example time is 10ms AND I compared both variant Co::usleep() & just usleep() with HOOKS it is have not same behaviour.

mrVrAlex commented 2 years ago

https://github.com/openswoole/swoole-src/blob/master/tests/swoole_runtime/sleep.phpt#L20 - if you change in this test Co::usleep to just usleep (hooks already enabled by Swoole\Runtime::enableCoroutine();) then test will fail.

doubaokun commented 2 years ago

@shandyDev we will look at this again later.

mrVrAlex commented 2 years ago

@doubaokun I think I found problem:

This is openswoole Co::usleep implementation:

PHP_METHOD(swoole_coroutine_system, usleep) {
    zend_long microseconds;

    ZEND_PARSE_PARAMETERS_START(1, 1)
    Z_PARAM_LONG(microseconds)
    ZEND_PARSE_PARAMETERS_END_EX(RETURN_FALSE);

    if (UNEXPECTED(microseconds < 0)) {
        php_swoole_fatal_error(E_WARNING, "Number of microseconds must be greater than or equal to " ZEND_TOSTR(0));
        RETURN_FALSE;
    }
    RETURN_BOOL(System::usleep(microseconds) == 0);
}

And this HOOK implementation:

static PHP_FUNCTION(swoole_usleep) {
    zend_long num;
    if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &num) == FAILURE) {
        RETURN_FALSE;
    }
    if (num < 0) {
        php_error_docref(nullptr, E_WARNING, "Number of seconds must be greater than or equal to 0");
        RETURN_FALSE;
    }
    double sec = (double) num / 1000000;
    if (Coroutine::get_current()) {
        System::sleep(sec);
    } else {
        usleep((unsigned int) num);
    }
}

Look there: double sec = (double) num / 1000000; it give double < 0.xxx value So in System::sleep(sec); double 0.xxx convert again to long with miss his double part:

int System::sleep(double sec) {
    return System::usleep((long) sec * 1000000);
} 

I suggest not casting chain zend_long -> double -> long and refactor HOOK implementation as same Co::usleep.