search

reactphp / socket

Async, streaming plaintext TCP/IP and secure TLS socket server and client connections for ReactPHP.
https://reactphp.org/socket/
MIT License
1.2k stars 156 forks source link

Memory leak in TcpServer due to PHP stream_context_create memory leak #303

Closed lucasnetau closed 1 year ago

lucasnetau commented 1 year ago

Hi @clue @WyriHaximus @SimonFrings , I wanted to bring to your attention an internal PHP memory leak (https://github.com/php/php-src/issues/10885) I found which has been affecting me with TcpServer (Specifically through react-child-process-messenger + react/filesystem).

Essentially for each instance of TcpServer, PHP was allocating 720+ bytes that was never released until shutdown, seems to have always existed since PHP4 days https://3v4l.org/XBaaP

The good news is the PHP team quickly provided a patch which has been merged into PHP8.1, 8.2, Master so the next releases will have a fix. It may be useful to add some documentation to TcpServer of this issue for memory constrained environments where there is a high churn of TcpServer instances.

WyriHaximus commented 1 year ago

So this only becomes an issue if you rapidly create temporary socket servers and shut them down again, correct? Also looking at that PHP issue, well done!

lucasnetau commented 1 year ago

@WyriHaximus Yes, In my use case the services are writing out a state file every 1 second. So each second there is a Filesystem putContents to a temp file followed by a rename. This is using Reach Filesystem Child Process, EIO is unusable per the previous tickets opened.

react-child-process-messenger makes good use of TcpServer to do the RPC work, so when doing file operations you rapidly leak memory.

WyriHaximus commented 1 year ago

@lucasnetau Thank you for confirming my thoughts on how this was leaking :+1:.

SimonFrings commented 1 year ago

@lucasnetau I agree with @WyriHaximus, thanks for your work on this one :+1:

In my opinion it is not necessary to add documentation for this to the TcpServer, this is not your everyday use case when it comes to creating and running a socket server, so it might be misleading to some users.

Great job on the memory leak tho!

SimonFrings commented 1 year ago

Update: The memory leak on PHP's side has been fixed via PHP 8.2.5 and PHP 8.1.18. This doesn't seem to be a bug in ReactPHP, so I will change the label of this ticket and close it, as it seems like everything in here has been answered.

Thanks again to @lucasnetau for reporting this :+1: