swoole / ext-postgresql

🐘 Coroutine-based client for PostgreSQL
64 stars 21 forks source link

Segmentation fault when run in distinct coroutine context #55

Open sergiotabanelli opened 3 years ago

sergiotabanelli commented 3 years ago

The following code produce a segmentation fault. The problem is that the registration of coroutine callbacks were done only in the connect method. I have a fork that fix this bug. I can do a pull request if needed.

<?php
$db;
\Co\run(function() use (&$db){
    $db = new \Swoole\Coroutine\PostgreSQL();
    $db->connect("host=127.0.0.1 port=5432 dbname=wireskel user=wireskel password=xxxxxx");
});
\Co\run(function () use ($db) {
    $res = $db->query('SELECT * from _cred_user'); // Segmentation Fault
});
sergiotabanelli commented 3 years ago

Hi, may I have a feedback? I have created a pull request that fix the bug. I think that run this extension in distinct coroutine contexts can be useful in unit testing and other things!

codercms commented 3 years ago

@sergiotabanelli hi! I think you shouldn't use Coroutine objects in this manner.

Coroutine::run - creates one global app event loop, application may have only one event loop at once. You trying to create two event loops in sequence. $db object belongs to your first event loop, but you trying to access $db from different event loop. But this case should never happen (for example: because Postgres socket is bound to the first event loop).

Your app arch should be like:

php main.php

// main.php content
<?php

// create event loop
\Co\run(function() {
    // you're inside event loop now

    $db = new \Swoole\Coroutine\PostgreSQL();
    $db->connect("host=127.0.0.1 port=5432 dbname=wireskel user=wireskel password=xxxxxx");

    \Swoole\Coroutine::create(function () use ($db) {
          $res = $db->query('SELECT * from _cred_user');
    });

    // Wait here for some events: e.g. coroutine exit or termination signal

    // disconnect from Postgres while you're in event loop
    unset($db);
});

exit(0);
sergiotabanelli commented 3 years ago

@codercms thanks for Your answer, but I have to disagree. IMHO: