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 http-proxy dependency when client closes connection prematurely #782

Open
2 tasks done
lbeschastny opened this issue May 24, 2022 · 10 comments
Open
2 tasks done

Comments

@lbeschastny
Copy link

lbeschastny commented May 24, 2022

Checks

Describe the bug (be clear and concise)

This is an issue of http-proxy, but I decided to report it here as well because:

  • http-proxy-middleware is affected by this issue
  • last http-proxy version was released in 2020, so I don't expect them to fix it soon
  • this issue could be fixed on http-proxy-middleware side

Here is a complete description of this issue: http-party/node-http-proxy#1586

Step-by-step reproduction instructions

Here is a minimal example to reproduce the above-mentioned memory leak:

const express = require('express');
const { createProxyMiddleware } = require('http-proxy-middleware');

const server = express()
	.use(createProxyMiddleware({ target: 'http://127.0.0.1:5000' }))
	.listen(7000);

server.once('listening', () => {
	console.log('server is listenning');
});

I used http-server to create an upstream server:

npx http-server . -p 5000

Memory leak appears when client sends a http request, but then closes connection prematurely after receiving HTTP headers without waiting for HTTP body.

I used cURL to simulate this behavior:

curl http://localhost:7000/15142e1388c685f57c58e0babbede1f1.jpg

Memory leak could be reproduces with cURL when requesting any binary file from a TTY terminal, since cURL doesn't support printing binary data to terminals and just closes the connection.

Expected behavior (be clear and concise)

Memory should not leak

How is http-proxy-middleware used in your project?

↪ npm ls http-proxy-middleware
culture-ru-gateway@1.0.6-alpha.0 /home/leonid/Documents/culture/gateway
└── http-proxy-middleware@2.0.6

What http-proxy-middleware configuration are you using?

{
	retry: {
		factor: 2,
		randomize: true,
		retries: 30,
		minTimeout: 10,
		maxTimeout: 500
	},
	xfwd: true,
	logLevel: 'warn'
}

What OS/version and node/version are you seeing the problem?

System:
    OS: Linux 5.13 Ubuntu 20.04.4 LTS (Focal Fossa)
    CPU: (8) x64 Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz
    Memory: 5.11 GB / 15.34 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 16.15.0 - ~/.nvm/versions/node/v16.15.0/bin/node
    npm: 8.5.5 - ~/.nvm/versions/node/v16.15.0/bin/npm
  Managers:
    Apt: 2.0.8 - /usr/bin/apt
    Cargo: 1.49.0 - ~/.cargo/bin/cargo
    pip3: 20.0.2 - /usr/bin/pip3
    RubyGems: 3.1.2 - /usr/bin/gem
  Utilities:
    CMake: 3.16.3 - /usr/bin/cmake
    Make: 4.2.1 - /usr/bin/make
    GCC: 9.4.0 - /usr/bin/gcc
    Git: 2.25.1 - /usr/bin/git
    Ninja: 1.10.0 - /usr/bin/ninja
    FFmpeg: 4.2.4 - /usr/bin/ffmpeg
  Servers:
    Nginx: 1.18.0 - /usr/sbin/nginx
  Virtualization:
    Docker: 20.10.12 - /usr/bin/docker
  IDEs:
    Nano: 4.8 - /usr/bin/nano
    Sublime Text: 4126 - /usr/bin/subl
    VSCode: 1.67.1 - /usr/bin/code
  Languages:
    Bash: 5.0.17 - /usr/bin/bash
    Perl: 5.30.0 - /usr/bin/perl
    Python3: 3.8.10 - /usr/bin/python3
    Ruby: 2.7.0 - /usr/bin/ruby
    Rust: 1.49.0 - /home/leonid/.cargo/bin/rustc
  Databases:
    MongoDB: 4.4.14 - /usr/bin/mongo
  Browsers:
    Chrome: 101.0.4951.64

Additional context (optional)

Here is a code I used to fix http-proxy memory leak in my project:

proxy.on('proxyRes', (proxyRes, req, res) => {
	const cleanup = (err) => {
		// cleanup event listeners to allow clean garbage collection
		proxyRes.removeListener('error', cleanup);
		proxyRes.removeListener('close', cleanup);
		res.removeListener('error', cleanup);
		res.removeListener('close', cleanup);

		// destroy all source streams to propagate the caught event backward
		req.destroy(err);
		proxyRes.destroy(err);
	};

	proxyRes.once('error', cleanup);
	proxyRes.once('close', cleanup);
	res.once('error', cleanup);
	res.once('close', cleanup);
});

The actual code could be simpler since:

  • it should be enough to listen only for 'close' events since they are always emitted after 'error' events (unless emitClose is false)
  • it's probably not necessary to call removeListener manually, because those listeners should not prevent streams from being garbage collected (but who knows, I certainly don't want to test it)
@audifaxdev
Copy link

Unable to reproduce locally and in staging/production. using node 14 LTS AND HPM 1.0.3

We have a software in production that has been running OK without any leak for weeks now, with millions request / day.

@lbeschastny
Copy link
Author

lbeschastny commented Sep 14, 2022

@audifaxdev I prepared a reproduction example here:
https://github.com/lbeschastny/http-proxy-memory-leak-example

You may run it locally or using docker-compose.
Or you could check my logs in Readme here.

Also, keep in mind that memory leak doesn't happen when all http requests are properly closed by clients.
That's why it may be difficult to reproduce this issue.
But I initially came upon it in our production environment, so it definitely can affect production servers.

@lbeschastny
Copy link
Author

I just added a hotfix branch to my reproduction example repository to demonstrate that it fixes the memory leak in question

@lbeschastny
Copy link
Author

Ideally, this issue should be addressed in http-proxy repository, not here.
But I don't see any activity there, so it may not happen anytime soon.

@alxlchnr
Copy link

I observed a similar problem and this issue helped me to get a clue of the reason behind leaking sockets. I would love a fix for this issue.

@Denoder
Copy link

Denoder commented Oct 2, 2022

Would you guys perhaps want to try to use my package?
https://github.com/refactorjs/http-proxy
https://www.npmjs.com/package/@refactorjs/http-proxy

it's basically http-proxy but incorporating most of the pull requests that looked useful.

@nestdimon
Copy link

Oh man, for a couple of days I struggled with a hanging SSE connection on browser tab refresh. Thanks for hotfix, it definitely should be merged!

@lbeschastny
Copy link
Author

Here is another fix for this memory leak: http-party/node-http-proxy#1559.

It was published to npm as http-proxy-node16.

@Muhammad-Ahmad-Rai
Copy link

@lbeschastny is this only an issue with node 15.15+, or is there memory leak in the older versions of node(node14) as well?

@lbeschastny
Copy link
Author

lbeschastny commented Jun 11, 2023

@Muhammad-Ahmad-Rai I just run my reproduction example lbeschastny/http-proxy-memory-leak-example using Node.js v14.15 and it worked fine (no memory leaks).

According to http-party/node-http-proxy#1559 this issue affects Node.js v15.5.0 and above.
I just run my reproduction example using Node.js v15.5 and it crushed with a memory leak.

I'm not sure about Node.js v15.x prior to v15.5.0, though.

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

No branches or pull requests

6 participants