Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MySqliDriver: Do not close connection when initialized with a resource (side effect) #411

Open
dakujem opened this issue Dec 14, 2021 · 0 comments

Comments

@dakujem
Copy link
Contributor

dakujem commented Dec 14, 2021

This is a feature request or an edge-case bug report.

MySqliDriver closes the connection upon destruction of its parent Connection object:
Connection::__destructConnection::disconnectMySqliDriver::disconnect (code here) ⟶ \mysqli::close

In a scenario like the following this behaviour will cause an error.

$container['database'] = new \mysqli($cfg['host'], $cfg['username'], $cfg['password'], $cfg['database']);

function doSomething(\mysqli $connection) {
    $dibi = new Connection(config: [
        'resource' => $connection,
        'driver' => 'mysqli',
    ]);
    $dibi->query(...);
    return;
    // at this point the $dibi variable goes out of scope and is destructed
}

/** @var \mysqli $db */
$db = $container->get('database');

// side-effect here, the DB disconnects after this function returns
doSomething($db);

// and then... oops!
$db->query(...); // Error: mysqli object is already closed

Even though this is an edge case, the scenario should be supported.
PdoDriver::disconnect() does not do this, it simply nulls the connection; I haven ot checked the other drivers.

Suggested solution

I suggest something like this:

// MySqliDriver
public function disconnect(): void {
    !$this->initializedWithResource && @$this->connection->close(); 
}

... and adding $this->initializedWithResource = true in the constructor here.

I am able to provide a PR.

P.S. I haven't tested or encountered this, but I noticed this possibility when working on #410

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant