Does mysqli::reap_async_query() has side effects?-CodePudding

I'm maintaining a project based on legacy code that was written by an external company. We are in the process of fixing a lot of PHP errors in this code base and one of them comes from a call to mysqli::reap_async_query() :

mysqli::reap_async_query(): Connection not opened, clear or has been closed

This happens only on the first call to this function:

    function mysqlQuery($sql, $getLastId = false, $die = true, $alwaysDie = false, $async = false)
    {
        @$GLOBALS['con']->reap_async_query(); // This is what is triggering the error
    
        if ($async) {
            $result = $GLOBALS['con']->query($sql, MYSQLI_ASYNC);
        } else {
            $result = $GLOBALS['con']->query($sql);
        }
        if (!$result) {
            if ($alwaysDie or ($_ENV['ENV_MODE'] === 'dev' and $die)) {
                die('Etwas stimmte mit dem Query nicht: ' . $GLOBALS['con']->error . '<br/>Ganzes Query: ' . $sql);
            }
            return false;
        } else {
            $lastId = mysqlLastId();
            if ($getLastId == true) {
                return $lastId;
            } else {
                return $result;
            }
        }
    }

Note that $GLOBALS['con'] is an instance of mysqli. The code is calling mysqlQuery() with no specific parameter:

$result = mysqlQuery("SELECT * FROM someTable");

According to the Git history, the call to @$GLOBALS['con']->reap_async_query(); was added to support async SQL queries. But reading the PHP doc, it doesn't seem to be useful here at all since we are not storing its return value.

So my question is: is there a reason for it to be here, does calling it even without reading its return value have any important side effect ? I might just remove this call completely if it is useless.

Also, why is it triggering this error ? I understand that trying to read a result before any query has been executed could trigger an error but the error indicates that the connection is not active, which does not seem to be the case.

CodePudding user response：

Is there a reason for it [mysqli::reap_async_query()] to be here, does calling it even without reading its return value has any important side effect ?

The return value is not assigned to a local variable, however it is still returned.

So the original interest was in calling that function. And for what for, has been written about in the commit message.

According to the Git history, the call to @$GLOBALS['con']->reap_async_query(); was added to support async SQL queries.

Let's consider this example:

$con->query('SELECT SLEEP(5) as `zzzZZZ...Schnarch...Schmatz..zzz`', MYSQLI_ASYNC);

$con->reap_async_query();

How long does it take to execute this code?

This is the reason how that call supports async queries. If an async query would still run on the connection (it has not been reaped yet), every new query would fail.

So add of the call in-so-far supports async SQL queries as it allows to fire one on the same (shared) connection that might be in use for other queries, too.

Additionally you ask:

Also, why is it triggering this error ? I understand that trying to read a result before any query has been executed could trigger an error but the error indicates that the connection is not active, which does not seem to be the case.

Let's take a look at the error, actually a message on the diagnostic channel:

PHP Warning: mysqli::reap_async_query(): Connection not opened, clear or has been closed in ...

As we know the connection has been opened and has not been closed, the last point might be it:

[...] Connection [...] clear [...]

Now I have not programmed that error message, but my reading of it is that there is no async query running yet on the (open) connection - the connection is clear.

It produces a warning as this might not be intended (there is no need to reap a clear connection normally) and henceforth as with your function this is intended, the call is prefixed with the error suppression operator (@).

CodePudding user response：

Thanks to @hakre's answer i understand that this call is there to get rid of the queue of async queries and i plan to fix it with a static flag to check if one is pending before calling @$GLOBALS['con']->reap_async_query():

function mysqlQuery($sql, $getLastId = false, $die = true, $alwaysDie = false, $async = false)
{
    static $asyncPending = false;

    if (true === $asyncPending) {
        @$GLOBALS['con']->reap_async_query();
        $asyncPending = false;
    }

    if ($async) {
        $result = $GLOBALS['con']->query($sql, MYSQLI_ASYNC);
        $asyncPending = true;
    } else {
        $result = $GLOBALS['con']->query($sql);
    }
    if (!$result) {
        if ($alwaysDie or ($_ENV['ENV_MODE'] === 'dev' and $die)) {
            die('Etwas stimmte mit dem Query nicht: ' . $GLOBALS['con']->error . '<br/>Ganzes Query: ' . $sql);
        }
        return false;
    } else {
        $lastId = mysqlLastId();
        if ($getLastId == true) {
            return $lastId;
        } else {
            return $result;
        }
    }
}