Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
lucasnetau opened this issue Mar 21, 2023 · 5 comments
Closed
Labels

Comments

@lucasnetau
Copy link

Hi @clue @WyriHaximus @SimonFrings , I wanted to bring to your attention an internal PHP memory leak (php/php-src#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.

@lucasnetau lucasnetau added the bug label Mar 21, 2023
@WyriHaximus
Copy link
Member

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
Copy link
Author

@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
Copy link
Member

@lucasnetau Thank you for confirming my thoughts on how this was leaking 👍.

@SimonFrings
Copy link
Member

@lucasnetau I agree with @WyriHaximus, thanks for your work on this one 👍

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 SimonFrings added question and removed bug labels Jun 13, 2023
@SimonFrings
Copy link
Member

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 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants