Open CodiMech25 opened 6 years ago
I've noticed this memory leak as well, i hope this will be merged with master.
@CodiMech25 @rleroi @curtiskelsey - I would like to locate the source of the problem if there is one. I run Ratchet in production and do not see these issues. Is it possible that you are taking a reference to $conn
somewhere and not releasing?
There is also no need to remove listeners if there are no references to $conn
itself as they will not prevent garbage collection of $conn.
@mbonneau I had no references of the $conn
object at all. It was a pure Ratchet use-case. In fact, i tested this on a Ratchet example defined in the docs with no custom modifications. Still same behavior.
Maybe this isn't a Ratchet problem, but a PHP problem. When i will have more time, i will test this on the latest PHP 7.3 and report then.
Finally had time to do the test again,
Test script In fact it is just a slightly modified version of the main example found in here.
<?php
require_once('dependencies/autoload.php');
use Ratchet\MessageComponentInterface;
use Ratchet\ConnectionInterface;
ini_set('memory_limit', '4M');
/**
* chat.php
* Send any incoming messages to all connected clients (except sender)
*/
class MyChat implements MessageComponentInterface {
protected $clients;
protected $memory_usage_before = null;
protected $conn_number = 0;
public function __construct() {
$this->clients = new \SplObjectStorage;
}
public function onOpen(ConnectionInterface $conn) {
$this->clients->attach($conn);
$memory_usage_now = memory_get_usage();
$this->conn_number++;
echo 'Connection ' . str_pad($this->conn_number, 3, '0', STR_PAD_LEFT) . ' | ';
echo 'Usage is: ' . $memory_usage_now . ' bytes';
if ($this->memory_usage_before !== null) {
echo ' (difference is: ' . (string)($memory_usage_now - $this->memory_usage_before) . ' bytes';
}
echo ' | Clients count: ' . (string)$this->clients->count();
echo PHP_EOL;
$this->memory_usage_before = $memory_usage_now;
}
public function onMessage(ConnectionInterface $from, $msg) {
foreach ($this->clients as $client) {
if ($from != $client) {
$client->send($msg);
}
}
}
public function onClose(ConnectionInterface $conn) {
$this->clients->detach($conn);
}
public function onError(ConnectionInterface $conn, \Exception $e) {
$conn->close();
}
}
// Run the server application through the WebSocket protocol on port 8080
$app = new Ratchet\App('127.0.0.1', 8080);
$app->route('/chat', new MyChat, array('*'));
$app->route('/echo', new Ratchet\Server\EchoServer, array('*'));
$app->run();
Output This is the entire output of the running script. On the other side (client) i've used simple JS solution and was constantly hitting the F5 button - so the connection will be dropped and new connection will be created.
PS ...> php .\test-git-issue.php
Connection 001 | Usage is: 1960928 bytes | Clients count: 1
Connection 002 | Usage is: 2049344 bytes (difference is: 88416 bytes | Clients count: 1
Connection 003 | Usage is: 2061544 bytes (difference is: 12200 bytes | Clients count: 1
Connection 004 | Usage is: 2073744 bytes (difference is: 12200 bytes | Clients count: 1
Connection 005 | Usage is: 2085944 bytes (difference is: 12200 bytes | Clients count: 1
Connection 006 | Usage is: 2098144 bytes (difference is: 12200 bytes | Clients count: 1
Connection 007 | Usage is: 2110984 bytes (difference is: 12840 bytes | Clients count: 1
.
.
.
Connection 157 | Usage is: 3976888 bytes (difference is: 12200 bytes | Clients count: 1
Connection 158 | Usage is: 3989088 bytes (difference is: 12200 bytes | Clients count: 1
Connection 159 | Usage is: 4001288 bytes (difference is: 12200 bytes | Clients count: 1
Connection 160 | Usage is: 4013488 bytes (difference is: 12200 bytes | Clients count: 1
PHP Fatal error: Allowed memory size of 4194304 bytes exhausted (tried to allocate 20480 bytes) in ...\dependencies\GuzzleHttp\Psr7\MessageTrait.php on line 158
Fatal error: Allowed memory size of 4194304 bytes exhausted (tried to allocate 20480 bytes) in ...\dependencies\GuzzleHttp\Psr7\MessageTrait.php on line 158
PHP version PHP 7.2.7 (cli) (built: Jun 19 2018 23:13:48) ( NTS MSVC15 (Visual C++ 2017) x64 ) PHP 7.3.1 (cli) (built: Jan 9 2019 22:22:34) ( NTS MSVC15 (Visual C++ 2017) x64 )
Ratchet version v0.4.1, last commit: 6bb65ff
So it seems, that the problem persists to this very day. The memory allocation difference is more or less the same for every created connection (12200 bytes) - i would assume, that's the memory needed to create a new connection class instance.
On the other hand, there are many reported PHP bugs related to the garbage collection, maybe are some of them related to this issue as well.
@mbonneau I hope this helps you a little :)
@CodiMech25 Great info! I was able to reproduce this.
This appears to be a garbage collection issue. When I ran your code as is, I had the same issue.
Adding gc_collect_cycles
here:
public function onClose(ConnectionInterface $conn) {
$this->clients->detach($conn);
gc_collect_cycles(); // <--------------------
}
Stopped the problem.
I hate calling gc_collect_cycles
to fix issues like this, but it does force it to release the memory, so there are not any resources being held inside Ratchet itself (in this case anyway).
Please let me know how it goes for you.
Just in case anyone is following along - here is a simple client for Chrome:
<!DOCTYPE html>
<html lang="en">
<body>
<script>
let ws = new WebSocket('ws://127.0.0.1:8080/chat');
ws.onopen = () => console.log('opened...');
</script>
</body>
</html>
Guys, read gc doc, please. It's not a bug, but implementation.
the cycle-finding algorithm as described above is executed whenever the root buffer runs full. The root buffer has a fixed size of 10,000 possible roots.
So it starts when possible roots buffer >= 10000. The point is we have the buffer on the one hand and 'memory_limit' on the other hand.
One way to solve memory issue is to calculate the number of 'memory_limit' of a particular app at the moment the buffer has reached the 10k limit. Then you can set 'memory_limit' a bit more than the number.
Another way is to add a timer, so that gc_collect_cycles() may be called in there:
$loop->addPeriodicTimer(300, function() {
gc_collect_cycles();
});
@mbonneau According to the comment from @proArtex it isn't PHPs fault. Like he said in the comment, this is the implementation of the garbage collection in PHP. So the only solution is to check the memory limit and usage somehow, and then accordingly call the garbage collection manually.
I've come with this solution:
class MyChat implements MessageComponentInterface {
private const MEMORY_USAGE_FOR_GC_COLLECTION = 75; // percentage
protected $clients;
protected $memory_usage_before = null; // @debug
protected $conn_number = 0; // @debug
private $memory_watch_dog;
public function __construct() {
$this->clients = new \SplObjectStorage;
$this->memory_watch_dog = new \stdClass();
$this->memory_watch_dog->max_memory = $this->getMemoryLimit();
$this->memory_watch_dog->recent_connections = 0;
$this->memory_watch_dog->check_interval = 1;
$this->memory_watch_dog->gb_collect_memory_usage = $this->memory_watch_dog->max_memory * (self::MEMORY_USAGE_FOR_GC_COLLECTION / 100);
$this->memory_watch_dog->last_memory_usage = null;
$this->memory_watch_dog->enabled = true;
}
public function onOpen(ConnectionInterface $conn) {
$this->clients->attach($conn);
if ($this->memory_watch_dog->enabled) {
// Record the created connection
if ($this->memory_watch_dog->recent_connections >= PHP_INT_MAX) {
$this->memory_watch_dog->recent_connections = 1;
} else {
$this->memory_watch_dog->recent_connections++;
}
$memory_usage = null;
// Check if garbage collection is necessary
if (
$this->memory_watch_dog->recent_connections === 1 ||
$this->memory_watch_dog->recent_connections % $this->memory_watch_dog->check_interval === 0
) {
if ($this->memory_watch_dog->last_memory_usage !== null) {
// Get the memory usage difference
$memory_usage = memory_get_usage();
$memory_usage_diff = $this->memory_watch_dog->last_memory_usage;
if ($memory_usage_diff > 0) {
$gc_collect_memory_usage = $this->memory_watch_dog->max_memory * (self::MEMORY_USAGE_FOR_GC_COLLECTION / 100);
$this->memory_watch_dog->check_interval = ($gc_collect_memory_usage - ($gc_collect_memory_usage % $memory_usage_diff)) / $memory_usage_diff;
if ($this->memory_watch_dog->check_interval <= 0) {
$this->memory_watch_dog->check_interval = 1;
}
if ($memory_usage >= $gc_collect_memory_usage) {
gc_collect_cycles();
}
} else {
$this->memory_watch_dog->enabled = false;
}
}
// Save current memory usage
$this->memory_watch_dog->last_memory_usage = ($memory_usage === null) ? memory_get_usage() : $memory_usage;
}
}
$memory_usage_now = memory_get_usage(); // @debug
$this->conn_number++; // @debug
echo 'Connection ' . str_pad($this->conn_number, 3, '0', STR_PAD_LEFT) . ' | '; // @debug
echo 'Usage is: ' . $memory_usage_now . ' bytes'; // @debug
if ($this->memory_usage_before !== null) { // @debug
echo ' (difference is: ' . (string)($memory_usage_now - $this->memory_usage_before) . ' bytes'; // @debug
} // @debug
echo ' | Clients count: ' . (string)$this->clients->count(); // @debug
echo PHP_EOL; // @debug
$this->memory_usage_before = $memory_usage_now; // @debug
}
public function onMessage(ConnectionInterface $from, $msg) {
foreach ($this->clients as $client) {
if ($from != $client) {
$client->send($msg);
}
}
}
public function onClose(ConnectionInterface $conn) {
$this->clients->detach($conn);
}
public function onError(ConnectionInterface $conn, \Exception $e) {
$conn->close();
}
private function getMemoryLimit() {
$ini_val = trim(ini_get('memory_limit'));
$val = substr($ini_val, 0, -1);
$unit = strtolower(substr($ini_val, -1));
switch($unit)
{
case 'g':
$val *= 1024 * 1024 * 1024;
break;
case 'm':
$val *= 1024 * 1024;
break;
case 'k':
$val *= 1024;
break;
}
return $val;
}
}
Here's the output:
PS D:\...\src> php .\test-git-issue.php
Connection 001 | Usage is: 1969264 bytes | Clients count: 1
Connection 002 | Usage is: 2057680 bytes (difference is: 88416 bytes | Clients count: 1
Connection 003 | Usage is: 2069880 bytes (difference is: 12200 bytes | Clients count: 1
Connection 004 | Usage is: 2082080 bytes (difference is: 12200 bytes | Clients count: 1
Connection 005 | Usage is: 2094280 bytes (difference is: 12200 bytes | Clients count: 1
.
.
.
Connection 089 | Usage is: 3134824 bytes (difference is: 12200 bytes | Clients count: 1
Connection 090 | Usage is: 2061968 bytes (difference is: -1072856 bytes | Clients count: 1
Connection 091 | Usage is: 2074168 bytes (difference is: 12200 bytes | Clients count: 1
.
.
.
Connection 178 | Usage is: 3135568 bytes (difference is: 12200 bytes | Clients count: 1
Connection 179 | Usage is: 2061968 bytes (difference is: -1073600 bytes | Clients count: 1
Connection 180 | Usage is: 2074168 bytes (difference is: 12200 bytes | Clients count: 1
.
.
.
Connection 199 | Usage is: 2306288 bytes (difference is: 12200 bytes | Clients count: 1
Connection 200 | Usage is: 2318488 bytes (difference is: 12200 bytes | Clients count: 1
Connection 201 | Usage is: 2330688 bytes (difference is: 12200 bytes | Clients count: 1
...
I've written the code in a way so the garbage collecting and memory usage isn't called with every request. It can be so much improved, but for the demonstration i think it's enough :)
This works for me just perfectly, however the question is, how much performance-expensive will it be. Maybe you could implement something like this to the Ratchets core and make it optional?
@CodiMech25 I still fault PHP - we shouldn't have to worry about garbage collection. How about internally running a gc_collect_cycles()
if we bump the ceiling before crashing? (I am sure there are many many discussions in other places about this idea, we don't need to see a discussion about that here though.)
More to this issue: This can be corrected without having to do the management as you are doing in your new script.
If Ratchet would remove cyclical references as it dereferences objects, they would be collected immediately instead of having to wait for PHP to get around to it. Just poking at Ratchet (in a haphazard, not-worrying-about-breaking-things way) for a few minutes, I was able to remove most of the memory growth. This is a change to Ratchet that I would support: finding these circular references and removing them as objects are dereferenced so that we don't have to rely on garbage collection to do it.
On the other hand, managing this issue by tracking memory usage and periodically running cycle collection is not something that will be implemented in Ratchet as it can be handled by the user and is not needed in most circumstances.
I have run Ratchet in production for years without this causing issues (on my memory tracking graph I do clearly see the sawtooth pattern of collection cycles though). If the process is run with enough of a memory ceiling, the garbage collector is able to prevent memory errors in most cases. If users require a tighter memory constraint then they will need to make their app more aware or possibly add a timer as @proArtex suggested.
Thank you for giving good details and background on the issue. This will be a good resource to point people to that are experiencing the same.
@mbonneau I agree with your conclusions, but still it would be nice to see some stand-alone memory management class included in the Ratchet resources (or maybe as an extension).
Anyways, i'm glad i could help :)
Hello ! This problem is still present in the new versions. Do you have any news regarding this topic @mbonneau ? Thanks !
@moafred The way to deal with this right now is here: https://github.com/ratchetphp/Ratchet/issues/662#issuecomment-454886034
I am still looking to see if I can dereference things so the memory can be cleared immediately but have been too busy to track down what that needs. Will let you know if I figure it out.
So, in the last 3-4 hours i was searching for a memory leak in my app. After a complete code-review of my application code i decided to make a new simple app built on Ratchet WS server and see, if the problem persists. And it did!
The problem was, that whenever i created a WS connection, the connection class instances built in the background of the Ratchet were not destroyed! So on every new connection was created a new connection class with event listeners and by dropping this connection on the client side, this background instance still exists. Because of this, my memory usage was something like this:
(i was running a PHP Ratchet WS server and testing from a JS client in the browser)
New connection Memory usage: 2.00 MB | Max. memory: 3.00 MB Connection dropped by client New connection Memory usage: 2.05 MB | Max. memory: 3.00 MB Connection dropped by client New connection Memory usage: 2.10 MB | Max. memory: 3.00 MB Connection dropped by client New connection Memory usage: 2.15 MB | Max. memory: 3.00 MB Connection dropped by client ... and so on, you get thw idea.
Then, of course, the PHP script throwed a "Max memory limit exceeded" error.
The fix for this is quite simple:
OLD Ratchet/src/Ratchet/Server/IoServer.php
NEW Ratchet/src/Ratchet/Server/IoServer.php
I think, you can even skip the $conn->removeAllListeners(); line, because the $conn class has no more instances at this time anyway.
So, now it seems to work pretty nice, no memory leak whatsoever.
But there is one thing i don't get, why isn't the garbage collector collecting things automatically when the memory limit is so near?