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

object of PostgreSQL() cannot be stored and used as class property through concurrent connections #183

Closed RedChops closed 2 years ago

RedChops commented 2 years ago

Please answer these questions before submitting your issue.

  1. What did you do? If possible, provide a simple script for reproducing the error.

Note: I'm using Laravel Octane for this experiment, but I'm not sure if that matters in this case.

I've noted that regardless of where the PostgreSQL client object is created in scope, if it's stored then accessed as a class property, swoole will crash. If I store it as a class property in the constructor, then assign it to a local variable in the function which will use it, that crashes swoole the same way.

It's important to note that this only happens with concurrent requests, such as using wrk, or the way I've been testing: xargs -I % -P 20 curl -s "http://localhost:8000/test" < <(printf '%s\n' {1..20}). If the request is made just from the browser, it works just fine.

Some basic tests:

public PostgreSQL $connection;

public function __construct()
{
    $this->connection = new PostgreSQL();
    if (!$this->connection->connect("host=pgsql;port=5432;dbname=database;user=sail;password=password")) {
        throw new Exception("Could not connect: " . $this->connection->error);
    }
}

public function __invoke()
{
    $queryId = 'z' . rand();

    if ($this->connection->prepare($queryId, "SELECT pg_sleep(2)") === false) {
        throw new Exception("Could not prepare: " . $this->connection->error);
    }

    if (($result = $this->connection->execute($queryId, [])) === false) {
        throw new Exception($this->connection->error, $this->connection->errCode);
    }

    if (($response = $this->connection->fetchAll($result)) === false) {
        if (!(in_array($this->connection->resultStatus, [0, 1, 2]))) {
            throw new Exception($this->connection->error, $this->connection->errCode);
        }
        $response = [];
    }
    return $response;
}

^Crashes

public PostgreSQL $connection;

public function __construct()
{
//        $this->connection = new PostgreSQL();
//        if (!$this->connection->connect("host=pgsql;port=5432;dbname=database;user=sail;password=password")) {
//            throw new Exception("Could not connect: " . $this->connection->error);
//        }
}

public function __invoke()
{
    $this->connection = new PostgreSQL();
    if (!$this->connection->connect("host=pgsql;port=5432;dbname=database;user=sail;password=password")) {
        throw new Exception("Could not connect: " . $this->connection->error);
    }
    $queryId = 'z' . rand();

    if ($this->connection->prepare($queryId, "SELECT pg_sleep(2)") === false) {
        throw new Exception("Could not prepare: " . $this->connection->error);
    }

    if (($result = $this->connection->execute($queryId, [])) === false) {
        throw new Exception($this->connection->error, $this->connection->errCode);
    }

    if (($response = $this->connection->fetchAll($result)) === false) {
        if (!(in_array($this->connection->resultStatus, [0, 1, 2]))) {
            throw new Exception($this->connection->error, $this->connection->errCode);
        }
        $response = [];
    }
    return $response;
}

^Also crashes

public PostgreSQL $connection;

public function __construct()
{
//        $this->connection = new PostgreSQL();
//        if (!$this->connection->connect("host=pgsql;port=5432;dbname=database;user=sail;password=password")) {
//            throw new Exception("Could not connect: " . $this->connection->error);
//        }
}

public function __invoke()
{
    $connection = new PostgreSQL();
    if (!$connection->connect("host=pgsql;port=5432;dbname=database;user=sail;password=password")) {
        throw new Exception("Could not connect: " . $connection->error);
    }
    $queryId = 'z' . rand();

    if ($connection->prepare($queryId, "SELECT pg_sleep(2)") === false) {
        throw new Exception("Could not prepare: " . $connection->error);
    }

    if (($result = $connection->execute($queryId, [])) === false) {
        throw new Exception($connection->error, $connection->errCode);
    }

    if (($response = $connection->fetchAll($result)) === false) {
        if (!(in_array($connection->resultStatus, [0, 1, 2]))) {
            throw new Exception($connection->error, $connection->errCode);
        }
        $response = [];
    }
    return $response;
}

^Works as intended

Another note: If I have another class create the client object and pass it to the __invoke() method, that also works as intended. The example of this is:

public function connect(array $params): PostgreSQL
{
    if (!isset(self::$pool)) {
        self::$pool = new ConnectionPool(
            fn(): PostgreSQL => $this->createConnection($this->dsn($params)),
            $params['poolSize'] ?? ConnectionPool::DEFAULT_SIZE,
        );
    }

    $connection = self::$pool->get();
    defer(static fn() => self::$pool->put($connection));
    return $connection;
}

/**
 * @throws ConnectionException
 */
public function createConnection(string $dsn): PostgreSQL
{
    $pgsql = new PostgreSQL();

    if (!$pgsql->connect($dsn)) {
        throw ConnectionException::failed($dsn);
    }

    return $pgsql;
}

Even though that is using a connection pool, if I call ->connect() from my __invoke() function, the requests all execute concurrently and without error.

  1. What did you expect to see?

I'd expect the results to all execute concurrently and without error regardless of where the PostgreSQL() client object is stored within a class.

  1. What did you see instead?

The Swoole worker itself actually crashes. In the worker output log, there is some number of entries like the following:

[2022-03-14 14:07:34 *3108.0]   WARNING ReactorEpoll::add(): failed to add events[fd=50#0, type=25, events=512], Error: File exists[17]
[2022-03-14 14:07:34 *3108.0]   WARNING ReactorEpoll::add(): failed to add events[fd=51#0, type=25, events=512], Error: File exists[17]
[2022-03-14 14:07:34 $2333.0]   WARNING Server::check_worker_exit_status(): worker(pid=3108, id=0) abnormal exit, status=0, signal=11
A bug occurred in OpenSwoole-v4.10.0, please report it.
The Swoole developers probably don't know about it,
and unless you report it, chances are it won't be fixed.
You can read How to report a bug doc before submitting any bug reports:
>> https://github.com/openswoole/swoole-src/blob/master/.github/ISSUE.md 
Please do not send bug reports in the mailing list or personal letters.
The issue page is also suitable to submit feature requests.

OS: Linux 5.10.60.1-microsoft-standard-WSL2 #1 SMP Wed Aug 25 23:20:18 UTC 2021 x86_64
GCC_VERSION: 10.2.1 20210110
OPENSSL_VERSION: OpenSSL 1.1.1k  25 Mar 2021
PHP_VERSION : 8.1.3

On a couple of occasions a PHP ErrorException was thrown with the message "swoole_event_add failed", which looks like it must come from https://github.com/openswoole/swoole-src/blob/4cf2ae4af2799ff344b4290103e0f72a9415acb5/ext-src/swoole_postgres_coro.cc#L776

To rule out any issues with WSL2, I ran this container in a pure Linux environment and had the exact same failures.

The requests also will hang curl, likely because of https://github.com/openswoole/swoole-src/issues/151

  1. What version of OpenSwoole are you using (show your php --ri openswoole)?
openswoole

Open Swoole => enabled
Author => Open Swoole Group <hello@openswoole.com>
Version => 4.10.0
Built => Mar 11 2022 14:24:47
coroutine => enabled with boost asm context
epoll => enabled
eventfd => enabled
signalfd => enabled
cpu_affinity => enabled
spinlock => enabled
rwlock => enabled
openssl => OpenSSL 1.1.1k  25 Mar 2021
dtls => enabled
pcre => enabled
zlib => 1.2.11
brotli => E16777225/D16777225
mutex_timedlock => enabled
pthread_barrier => enabled
futex => enabled
async_redis => enabled
postgresql => enabled

Directive => Local Value => Master Value
swoole.enable_coroutine => On => On
swoole.enable_library => On => On
swoole.enable_preemptive_scheduler => Off => Off
swoole.display_errors => On => On
swoole.use_shortname => On => On
swoole.unixsock_buffer_size => 8388608 => 8388608
  1. What is your machine environment used (show your uname -a & php -v & gcc -v) ?

uname: Linux 50d1a3fdb76e 5.10.60.1-microsoft-standard-WSL2 #1 SMP Wed Aug 25 23:20:18 UTC 2021 x86_64 GNU/Linux Linux xxx 5.10.0-11-amd64 #1 SMP Debian 5.10.92-1 (2022-01-18) x86_64 GNU/Linux

php: PHP 8.1.3 (cli) (built: Mar 11 2022 02:58:22) (NTS) Copyright (c) The PHP Group Zend Engine v4.1.3, Copyright (c) Zend Technologies with Zend OPcache v8.1.3, Copyright (c), by Zend Technologies

gcc is not installed in the Docker container.

RedChops commented 2 years ago

Okay, this seems to have something to do with Octane. If I create the test class above and invoke it from a minimal server.php and Swoole HTTP server setup, I don't hit this issue.

Thinking it might have something to do with Laravel's heavy use of Reflection, I tried to declare the property dynamically. This doesn't change the situation unfortunately.

RedChops commented 2 years ago

I discovered the source of this issue, but I don't really know the best way to solve it totally. I have a solution that works in my project.

In my first two cases, the requests fail for two different reasons. Because Octane does not run a controller constructor for consecutive requests, the PostgreSQL object stored as an instance variable is either used twice concurrently, failing because the file descriptor is already in use, or overwritten by each request in which case a similar issue appears. I discovered this issue while writing a Laravel database driver for this client, so my failure was pretty similar to the test cases in the end.

So basically, if the PostgreSQL client as it stands today is used within a context managed by Laravel/Octane, where the context may be stored or optimized between requests, the client cannot be stored as an instance variable because it cannot detect if it should be using a new coroutine context or not.

I don't know if there is anything to actually fix or improve here. If not, you can feel free to close this ticket.

doubaokun commented 2 years ago

PostgreSQL object can't be used concurrently, so you can create a pool of clients and borrow and return when using within the controller.