[5.4] Fix 'Too Many Connections' error during testing using $connectionsToTransact

[twig1337]

Terminate user defined database connections after rollback while using the DatabaseTransactions trait.

Note: This bug fix is pointed to the 5.4 branch as the corresponding code doesn't exist in the 5.1 release.

[thecrypticace]

I run into this one all the time. The point where I created a trait that does this (but does so by overriding tearDown by keeping temp variables around to be able to call setPDO(null) on it afterwards).

Never noticed connection had a disconnect method. I quite like this approach. How well does it work when used with the DatabaseMigrations trait?

[taylorotwell]

What causes the too many connections? Obviously this doesn't happen on every test suite. Is adding this going to seriously affect performance of the tests?

[themsaid]

Maybe related: #18471

[thecrypticace]

What causes the too many connections?

I think something is holding onto the connection object. Maybe there's a reference cycle that's not being cleaned up?
Thinking out loud: I wonder if running gc_collect_cycles() would fix anything…

… Obviously this doesn't happen on every test suite

Most likely because:

1. Most test suites use SQLite for testing instead of MySQL
2. The default concurrent connection count for MySQL is 300 connections
3. I'd guess its unlikely that there'd be 300+ test cases that all use the database for at least smallish apps.

If you have an application that connects to more than one database (I have one that uses 4 for instance) it makes this problem pop up much, much quicker.

[twig1337]

In my case I am using MySql for the testing.
As @thecrypticace suggested my 'max_connections' value is 151.
I have 5 database connections that are created on every request to my API.
I have a suite of 36 tests.
36 * 5 = 180 = 💥

I'm not sure where the default 'mysql' connection is terminated, but upon testing it appears that it is terminated after each test is ran, preventing this sort of issue from occurring if a user is not using the $connectionsToTransact variable. But if you are none of the connections are terminated until after the entire test suite is completed.

[twig1337]

@taylorotwell If I increase my max_connections value and run my test suite without my changes the elapsed time, according to PHPUnit, is 33 seconds.
If I run my test suite with my changes it is 31 seconds.

So these changes actually increased performance in my scenario, which makes sense because my database isn't trying to maintain hundreds of connections simultaneously.

[twig1337]

I would also think that any proponent of TDD would say that my suite of 36 tests is child's play, not suitable as a true testing solution. In fact the tests in question are only my acceptance tests, this issue would be greatly exacerbated if I include integration tests across components that reference multiple database connections.

[thecrypticace]

Edit: never mind using $app->flush() like the base test class does solves the issue with the code below. Back to investigating

---

I did a bit of testing. I can reproduce the error with this script:

require __DIR__.'/bootstrap/autoload.php';

for ($i = 0; $i < 300; $i++) {
    $app = require __DIR__.'/bootstrap/app.php';
    $app->make(Illuminate\Contracts\Console\Kernel::class)->bootstrap();
    $app["db"]->connection(null)->beginTransaction();
    $app["db"]->connection(null)->rollBack();
    $app = null;

    echo ".";
}

And it appears that gc_collect_cycles(); does not help.

[mfn]

Many thanks for the PR! Can also confirm it works.

I feel a bit bad because I was using a custom trait with exactly this changes for years but failed to provide an upstream PR :-/

[denitsa-md]

I am still having this problem in Laravel 5.6.8 with the $connectionsToTransact property in a base test case (with 2 connections) and a mysql database.