Database reconnecting logic broken when using DB::purge()

[stancl]

Laravel Version

11.x

PHP Version

any

Database Driver & Version

irrelevant

Description

I've discovered this while debugging a very confusing scenario with parallel tests. It may be best if I start with that before getting to the root issue here. I apologize for the length/convoluted report upfront.

Essentially, if you use CACHE_DRIVER=database in tests, put this in AppServiceProvider::boot():

RateLimiter::for('anything', function () { /* doesn't matter */ });

and try to run tests with -p, while using cache in your tests, you will get "Call to a member function prepare() on null" aka somewhere we tried to use a Connection instance whose $pdo is set to null.

The cause is that RateLimiter instantiates Cache\Repository, which in the case of CACHE_DRIVER=database contains DatabaseStore which then holds a Connection instance. You can verify this by simply doing cache()->store('database'); in the service provider instead of using RateLimiter.

So you basically get CacheManager::$stores['database'] = DatabaseStore with a concrete Connection injected.

This then gets invalidated by this call

framework/src/Illuminate/Testing/Concerns/TestDatabases.php

Line 53 in 8ffd13c

$this->switchToDatabase($testDatabase);

calling DB::purge(), only when parallel tests are used.

After some source diving I found that connections should be self-healing, via the Connection::reconnect() method. However, this logic seems to be flawed (?) because it only creates a new repaired Connection on DatabaseManager but doesn't fix the original Connection's $pdo.

See this:

framework/src/Illuminate/Database/Connection.php

Lines 765 to 771 in 8ffd13c

	protected function run($query, $bindings, Closure $callback)
	{
	foreach ($this->beforeExecutingCallbacks as $beforeExecutingCallback) {
	$beforeExecutingCallback($query, $bindings, $this);
	}
	$this->reconnectIfMissingConnection();

framework/src/Illuminate/Database/Connection.php

Lines 1006 to 1011 in 8ffd13c

	public function reconnectIfMissingConnection()
	{
	if (is_null($this->pdo)) {
	$this->reconnect();
	}
	}

framework/src/Illuminate/Database/Connection.php

Lines 992 to 996 in 8ffd13c

	public function reconnect()
	{
	if (is_callable($this->reconnector)) {
	return call_user_func($this->reconnector, $this);
	}

With $reconnector being:

framework/src/Illuminate/Database/DatabaseManager.php

Lines 71 to 73 in 8ffd13c

	$this->reconnector = function ($connection) {
	$this->reconnect($connection->getNameWithReadWriteType());
	};

My suggestion is that $reconnector should look more something like this?

$this->reconnector = function ($connection) {
    $connection->setPdo(
        $this->reconnect($connection->getNameWithReadWriteType())->getRawPdo()
    );
};

(One caveat being that we don't deal with read/write PDO granularity here but I'm not sure if the reconnect logic does at all anyway.)

With that change, the issues described above disappear since DatabaseStore's $connection gets healed by the reconnect logic.

For now I've created a draft PR #53693.

Update: I've removed a section from here with some concerns, where I was worried that I may be missing the context for the current behavior. More source diving made it clear that the reconnecting logic makes perfect sense with DB::disconnect() but there's likely an oversight when the combination of DB::purge() and holding the purged Connection somewhere is used.

Thanks for your time.

Steps To Reproduce

$connection = DB::connection();

dump($connection->table('users')->get());

DB::purge();

// Call to a member function prepare() on null (= using a Connection instance w/ null $pdo)
dump($connection->table('users')->get());

[stancl]

Seems like the reconnector logic has been the same for 10 years since it was originally added in 143f7a9

Going over the code a bit more, my understanding of how the reconnector logic makes sense currently is:

1. DB::disconnect() keeps the Connection instance in DatabaseManager but clears its PDO
2. DB::purge() clears the PDO AND removes the Connection instance from the DB manager

My best guess would be that the reconnecting logic works fine if you only use a single Connection instance because then the connection that triggers the reconnect due to missing a $pdo value will be the same as the one within the DB manager that gets healed.

However if you use DB::purge() but manage to keep the old connection alive somewhere, that's when the problem occurs.

With that, I think it makes sense that the logic not working with DB::purge() is just an oversight, and there shouldn't be any issues with applying my patch, like what I mentioned above with the potentially problematic dynamically created connections.

[stancl]

I've removed a section with some concerns from the original post. After discovering that the reconnecting logic makes sense with DB::disconnect() and there's a problem only with the combination of DB::purge() and keeping the old Connection around, it makes sense that my understanding here is correct and there's just an oversight in the implementation that affects this specific case.

[github-actions]

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!