Some traits of database tests do not close the connection even after the test is finished

[KentarouTakeda]

Laravel Version

10.35.0

PHP Version

8.3.0

Database Driver & Version

PostgreSQL 16.1 for MacOS(MacPorts) / Probably all databases

Description

DatabaseMigrations and DatabaseTruncation do not close the database connection at the end of each test. Therefore, the database connection remains open for the number of tests.

As a result, when running a large number of tests, we may encounter a maximum number of connections error or a lack of resources on the database side.

DatabaseTransactions and RefreshDatabase do not have this problem.

Steps To Reproduce

In the following example, I adjusted the PostgreSQL config to make it easier to see what happens. It probably happens with other databases as well.

1. Start the server with postgresql.conf set as follows.

   max_connections = 3 # intentionally small value
   log_connections = on
   log_disconnections = on

2. Run the following test:

   use DatabaseTruncation; // or `DatabaseMigrations`
   
   public static function repeat(): array
   {
        // Run the test more times than the maximum number of concurrent connections
        return array_fill(0, 4, []);
   }
   
   /** @dataProvider repeat */
   public function test(): void
   {
   }

3. The test will fail with the following error for example:

   SQLSTATE[08006] [7] connection to server on socket "/tmp/.s.PGSQL.5432" failed: FATAL: sorry, too many clients already (Connection: pgsql, SQL: SET CONSTRAINTS ALL DEFERRED;)
   

4. The following is recorded in the database server log:

   LOG: connection received: host=[local]
   LOG: connection authorized: user=postgres database=postgres
   LOG: connection received: host=[local]
   LOG: connection authorized: user=postgres database=postgres
   LOG: connection received: host=[local]
   LOG: connection authorized: user=postgres database=postgres
   LOG: connection received: host=[local]
   FATAL: sorry, too many clients already
   LOG: connection received: host=[local]
   FATAL: sorry, too many clients already
   LOG: disconnection: session time: 0:00:00.012 user=postgres database=postgres host=[local]
   LOG: disconnection: session time: 0:00:00.024 user=postgres database=postgres host=[local]
   LOG: disconnection: session time: 0:00:00.058 user=postgres database=postgres host=[local]
   

• In the case of DatabaseTransactions and RefreshDatabase, the connection is closed after each test as per their implementation, so no error occurs and the log is recorded as follows.

  LOG: connection received: host=[local]
  LOG: connection authorized: user=postgres database=postgres
  LOG: disconnection: session time: 0:00:00.034 user=postgres database=postgres host=[local]
  LOG: connection received: host=[local]
  LOG: connection authorized: user=postgres database=postgres
  LOG: disconnection: session time: 0:00:00.030 user=postgres database=postgres host=[local]
  LOG: connection received: host=[local]
  LOG: connection authorized: user=postgres database=postgres
  LOG: disconnection: session time: 0:00:00.030 user=postgres database=postgres host=[local]
  LOG: connection received: host=[local]
  LOG: connection authorized: user=postgres database=postgres
  LOG: disconnection: session time: 0:00:00.029 user=postgres database=postgres host=[local]
  LOG: connection received: host=[local]
  LOG: connection authorized: user=postgres database=postgres
  LOG: disconnection: session time: 0:00:00.031 user=postgres database=postgres host=[local]
  

[mpyw]

Related to #18056. Either DatabaseMigrations or DatabaseTruncation doesn't implement disconnection logic. Note that it reproduces also when we don't any traits.

Trait	Connection leaks?
RefreshDatabase	🆗
DatabaseTransactions	🆗
DatabaseTruncation	❌
DatabaseMigrations	❌
Using DB with no traits	❌

[crynobone]

You may try to use add the following code in Tests\TestCase

    protected function setUp(): void
    {
        $this->beforeApplicationDestroyed(function () {
            foreach (array_keys($this->app['db']->getConnections()) as $name) {
                $this->app['db']->purge($name);
            }
        });

        parent::setUp();
    }

We have this in Framework's DatabaseMigrations tests without any side-affect, best to try this first before making any changes to the Framework itself.

[mpyw]

@crynobone I've already tried and I confirmed it solves this problem, but I believe that it should be implemented on framework core.

[crynobone]

beforeApplicationDestroyed runs callbacks in a sequence, if you have multiple beforeApplicationDestroyed and it uses db after the connection gets purged it "may" cause breaking change.

[mpyw]

At least we should update the trait doc comments and Laravel official documentation...

[mpyw]

@taylorotwell Hi, how do you think about this?

[driesvints]

We'd appreciate a PR here, thanks.

[mpyw]

@driesvints Please re-open this, there's still a problem

[KentarouTakeda]

@taylorotwell @driesvints

First of all, I would like to apologize for breaking the real world application.

However, I think this issue should be fixed.

In tearDown:

• It looks like you are trying to completely discard the application settings and class state. https://github.com/laravel/framework/blob/v10.37.3/src/Illuminate/Foundation/Testing/TestCase.php#L198-L204
• And very recently a commit was made that does it more precisely.
  bb49a72

The above process is being performed. I find it strange that the database connection is left open (and/or leaked). Is there some reason?

[mpyw]

@taylorotwell @driesvints @KentarouTakeda @rikvdh @crynobone I opened the discussion, please check it out:

• [Tests] How do we fix connection leaks when using `DatabaseTruncation` or `DatabaseMigrations`? #49373