[6.x/7.x] broadcast() method causes memory leak

[rennokki]

Like the title says, I ran into a discussion about some memory leaks for beyondcode/laravel-websockets here: beyondcode/laravel-websockets#325

The basic truth is that after some testing, I found out that running broadcast(...) will severely increase the memory for a ReactPHP loop, for instance.

To replicate this:

• create a new Laravel project
• composer require laravel/websockets:"dev-2.x-memory-leak"
• copy the migrations: php artisan vendor:publish --provider="BeyondCode\LaravelWebSockets\WebSocketsServiceProvider" --tag="migrations"
• run them: php artisan migrate
• get to the root of the project and run php artisan websockets:serve --statistics-interval=1
• open an artisan server with php artisan serve
• Go to http://127.0.0.1:8000/laravel-websockets
• Click on Connect
• Watch the memory increase in the console that ran websockets:serve

As explained in beyondcode/laravel-websockets#325 (comment) and beyondcode/laravel-websockets#325 (comment), the problem starts from a broadcast() that causes the leakage.

I have tested even the code within the event. Running broadcast() caused the memory leaks.

[taylorotwell]

What driver is broadcast using? Redis?

[taylorotwell]

It would need to be tracked down if this is something that actually happens in Laravel. It could be in the Redis library you are using, etc.

[rennokki]

It's using the native Redis broadcast driver from Laravel.

[taylorotwell]

Predis or phpredis?

[rennokki]

I think predis, if that's the one that is set by default within homestead

[GrahamCampbell]

Can you please find out the actual driver, with 100% certainty.

[rennokki]

Predis.

[rennokki]

@GrahamCampbell @taylorotwell For 2.0.0-beta.8 and afterward, I have removed the broadcast() method from the code to avoid memory leaking while testing for 2.0, so make sure you get the 2.0.0-beta.7 only.

[GrahamCampbell]

And the issue only occurs with predis, or also with phpredis? phpredis is the recommended driver, with up to 7x better performance.

[rennokki]

So, to give you the full console output for each driver:

PHPRedis:

Click for logs

➜  code php artisan websockets:serve --statistics-interval=1
"Using client: phpredis"
Starting the WebSocket server on port 6001...
Using: 18.32 MB
Saving statistics...
Using: 18.34 MB
Saving statistics...
Using: 18.34 MB
Saving statistics...
Using: 18.34 MB
Saving statistics...
Using: 18.34 MB
[ HERE I HAVE INITIATED A NEW CONNECTION; UNTIL AT THIS POINT, THE broadcast() DID NOT GET CALLED BECAUSE THERE WERE NO CONNECTED USERS ]
Using: 19.88 MB
Saving statistics...
Using: 19.98 MB
Saving statistics...
Using: 20.09 MB
Saving statistics...
Using: 20.19 MB
Saving statistics...
Using: 20.3 MB
Saving statistics...
Using: 20.4 MB
Saving statistics...
[ HERE I HAVE DISCONNECTED FROM THE WEBSOCKETS SERVER ]
Using: 20.51 MB
Saving statistics...
Using: 20.62 MB
Saving statistics...
Using: 20.72 MB
Saving statistics...
Using: 20.83 MB
Saving statistics...
Using: 20.95 MB
Saving statistics...
Using: 21.06 MB
Saving statistics...
Using: 21.16 MB
Saving statistics...
Using: 21.27 MB
Saving statistics...
Using: 21.37 MB

Predis:

Click for logs

➜  code php artisan websockets:serve --statistics-interval=1
"Using client: predis"
Starting the WebSocket server on port 6001...
Using: 18.64 MB
Saving statistics...
Using: 18.66 MB
Saving statistics...
Using: 18.66 MB
Saving statistics...
Using: 18.66 MB
Saving statistics...
Using: 18.66 MB
Saving statistics...
Using: 18.66 MB
Saving statistics...
Using: 18.66 MB
Saving statistics...
Using: 18.66 MB
Saving statistics...
Using: 18.66 MB
[ HERE I HAVE INITIATED A NEW CONNECTION; UNTIL AT THIS POINT, THE broadcast() DID NOT GET CALLED BECAUSE THERE WERE NO CONNECTED USERS ]
Using: 20.2 MB
Saving statistics...
Using: 20.3 MB
Saving statistics...
Using: 20.41 MB
Saving statistics...
Using: 20.51 MB
Saving statistics...
Using: 20.62 MB
Saving statistics...
Using: 20.72 MB
Saving statistics...
Using: 20.83 MB
Saving statistics...
Using: 20.94 MB
Saving statistics...
Using: 21.04 MB
Saving statistics...
Using: 21.15 MB
Saving statistics...
Using: 21.27 MB
Saving statistics...
[ HERE I HAVE DISCONNECTED FROM THE WEBSOCKETS SERVER ]
Using: 21.34 MB
Saving statistics...
Using: 21.45 MB
Saving statistics...
Using: 21.55 MB
Saving statistics...
Using: 21.66 MB
Saving statistics...
Using: 21.76 MB
Saving statistics...
Using: 21.87 MB
Saving statistics...
Using: 21.97 MB
Saving statistics...
Using: 22.08 MB
Saving statistics...
Using: 22.18 MB
Saving statistics...
Using: 22.29 MB
Saving statistics...
Using: 22.4 MB
Saving statistics...
Using: 22.5 MB
Saving statistics...
Using: 22.61 MB
Saving statistics...

So it doesn't depend on which driver is used.

[GrahamCampbell]

Thanks.

[rennokki]

@taylorotwell @GrahamCampbell I can confirm this memory leak happens even on the sync driver (so no queues involved). Either the ShouldBroadcastNow contract does not change the game at all.

[rennokki]

I have pointed out in beyondcode/laravel-websockets#325 (comment) that a controller also caused the issue, eventually moving on in beyondcode/laravel-websockets#325 (comment) to find out the issue is because of the broadcasting dispatch.

The package got a dev-2.x-memory-leak version in composer where just the broadcast() method causes the memory leak.

[rennokki]

The issue seems to be caused by the new StatisticsUpdated(...) bit, where StatisticsUpdated is an event class.

Calling broadcast(null); triggers no memory leak.

[mo3000]

I followed the instructions step by step to reproduce this. After digging a while, I found out that in /facade/ignition/src/IgnitionServiceProvider.php, function boot():

    public function boot()
    {
    ...
    
//        $this->app->make(QueryRecorder::class)->register();
//        $this->app->make(LogRecorder::class)->register();
//        $this->app->make(DumpRecorder::class)->register();
    }

comment out these three lines make the memory leak go away, and the websocket part works well.
So it sounds like not a bug of "broadcast()".

[taylorotwell]

See above.

[mpskovvang]

If the memory leak is caused by running SQL queries with many bindings, a fix is to set reporting.maximum_number_of_collected_queries to 0 in config/flare.php.

[rennokki]

@mpskovvang Makes sense! I have informed the package users that had the respective issue.