Backpressure not applied to HTTP/1.x client responses

[bileslav]

Hello. I need your help in finding a memory leak that appears to be somewhere in amphp/http-server:v3.0.0-beta.2 (not 100% that it's there). I'm assuming this because my application works correctly on the previous Amp version (which is without the wonderful fibers) and this is the second time I've run into this problem.

I should mention that I made one change in the vendor folder:
private const ALPN = ['http/1.1'];@DefaultHttpDriverFactory.php

I needed to disable HTTP/2 support on the server, and I didn't find a similar setting elsewhere. This is actually a topic for separate issue (question or feature request).

So. My application reaches out of memory (2G) for two to three minutes. The server is engaged in HTTP-streaming of mp4. I've tried ext-meminfo and some other tools, without success. I am willing to provide any diagnostic data.

[bileslav]

I found something.

use Amp\Http\{Client, Server};

require 'vendor/autoload.php';

$client = Client\HttpClientBuilder::buildDefault();
$request_handler = function (Server\Request $request) use ($client): Server\Response {
	$headers = [
		'Range' => $request->getHeader('Range'),
	];

	$request = new Client\Request('https://geometryx.ru/sample.mp4', 'GET');
	$request->setProtocolVersions(['1.1']);
	$request->setHeaders($headers);
	$request->setBodySizeLimit(PHP_INT_MAX);
	$request->setTransferTimeout(3600);

	$response = $client->request($request);

	return new Server\Response(
		$response->getStatus(),
		$response->getHeaders(),
		$response->getBody(),
	);
};

$request_handler = new Server\RequestHandler\ClosureRequestHandler($request_handler);

$server = new Server\SocketHttpServer(new Psr\Log\NullLogger());
$server->expose(new Amp\Socket\InternetAddress('127.0.0.1', 1024));
$server->start($request_handler, new Server\DefaultErrorHandler());

Amp\trapSignal(SIGINT);
$server->stop();

This line: $request->setBodySizeLimit(PHP_INT_MAX);

It helps when I comment it. I looked where this limit is used, and it remains a mystery to me how this causes the leak.

However, the memory seems to continue to leak, but now slowly. I continue to investigate.

[kelunik]

So you're proxying requests using http-client? Do you know whether the leak is inside http-client or the server?

[bileslav]

@kelunik,

So you're proxying requests using http-client?

Yes.

Do you know whether the leak is inside http-client or the server?

As I mentioned in the update above, the issue is with the Request::setBodySizeLimit(). But I could not understand what and in which of the packages happens next. There's no more information at the moment.

Are you able to reproduce the leak with my example? It assumes the default memory limit (128M). You need to start the server and open the page in the browser. Almost immediately, it will fall over the limit, but only if you don't comment out that line.

[trowski]

@bileslaw I pushed a commit, d61d8e6, that adds an option to disable HTTP/2 in DefaultHttpDriverFactory.

I can reproduce the issue locally, though didn't yet investigate what's happening. If you manage to find something let me know, otherwise I'll have a look this weekend.

[bileslav]

@trowski, I switched back to v2. So far, I cannot call my software complete and debugged. I decided to focus on that, and when I'm done it will make sense to port it to v3, as we will have a reference in that case.

Thank you for your reply and for adding the option!

[bileslav]

I completely switched to v3 but had to replace amphp/http-client with my own implementation over curl_multi (works great in conjunction with amp), and amphp/http-server with own implementation over amphp/socket + nginx. Haven't seen any memory leaks since.

[kelunik]

@bileslaw curl_multi won't work great in conjunction with Amp, because it has it's own event loop and there can't be two event loops running efficiently at the same time. It's not a memory leak, but rather a backpressure issue. The proxy downloads the video faster than the browser requests it, so memory usage grows. This has been fixed in amphp/http-client@e89e33e.

[bileslav]

@kelunik,

curl_multi won't work great in conjunction with Amp, because it has it's own event loop and there can't be two event loops running efficiently at the same time.

Yes, I picked the wrong word. Since the solution has to use polling (I stopped at 250 curl_multi_exec() calls per second), that is, due to the reason you mentioned, the solution cannot be called great. However, I've tested the client extensively, and it's been running flawlessly in production for a while now. I could share it with you if you're interested.

It's not a memory leak, but rather a backpressure issue.

I believe the commit you pushed solves some problem, but not the one I wrote about here.

The proxy downloads the video faster than the browser requests it, so memory usage grows.

I took this into account, it's not the case.

---

I will try Amp HTTP packages again later.

[bileslav]

Okay, I've been trying it for a few hours now. Everything works well! Of course, more time is needed, but I think it's okay if I close the issue now so that it doesn't distract anyone.

[kelunik]

No, I've reopened on purpose, there's still something strange that shouldn't behave like it currently behaves.

[bileslav]

Oh, okay. Are you talking about the server (I guess this, but it's worth clarifying) or the client? And could you please briefly describe the problem?

[bileslav]

[18-Aug-2022 00:35:25 Europe/Moscow] PHP Fatal error:  Uncaught Exception: Fiber stack protect failed: mprotect failed: Cannot allocate memory (12) in vendor/revolt/event-loop/src/EventLoop/Internal/AbstractDriver.php:495
Stack trace:
#0 vendor/revolt/event-loop/src/EventLoop/Internal/AbstractDriver.php(495): Fiber->start()
#1 vendor/revolt/event-loop/src/EventLoop/Internal/AbstractDriver.php(549): Revolt\EventLoop\Internal\AbstractDriver->invokeCallbacks()
#2 [internal function]: Revolt\EventLoop\Internal\AbstractDriver->Revolt\EventLoop\Internal\{closure}()
#3 vendor/revolt/event-loop/src/EventLoop/Internal/AbstractDriver.php(83): Fiber->resume()
#4 vendor/revolt/event-loop/src/EventLoop/Internal/DriverSuspension.php(95): Revolt\EventLoop\Internal\AbstractDriver->Revolt\EventLoop\Internal\{closure}()
#5 vendor/amphp/amp/src/functions.php(116): Revolt\EventLoop\Internal\DriverSuspension->suspend()
#6 src/main.php(140): Amp\trapSignal()
#7 bin/voxy(11): main()
#8 {main}
  thrown in vendor/revolt/event-loop/src/EventLoop/Internal/AbstractDriver.php on line 495
[18-Aug-2022 00:35:56 Europe/Moscow] PHP Fatal error:  Uncaught Error: Queue has already been completed, define environment variable AMP_DEBUG or const AMP_DEBUG = true and enable assertions for a stacktrace of the previous resolution. in vendor/amphp/pipeline/src/Internal/QueueState.php:349
Stack trace:
#0 vendor/amphp/pipeline/src/Internal/QueueState.php(327): Amp\Pipeline\Internal\QueueState->finalize()
#1 vendor/amphp/pipeline/src/Queue.php(119): Amp\Pipeline\Internal\QueueState->error()
#2 vendor/amphp/http-client/src/Connection/Internal/Http2ConnectionProcessor.php(1333): Amp\Pipeline\Queue->error()
#3 vendor/amphp/http-client/src/Connection/Internal/Http2ConnectionProcessor.php(1427): Amp\Http\Client\Connection\Internal\Http2ConnectionProcessor->releaseStream()
#4 vendor/amphp/http-client/src/Connection/Internal/Http2ConnectionProcessor.php(174): Amp\Http\Client\Connection\Internal\Http2ConnectionProcessor->shutdown()
#5 vendor/amphp/http-client/src/Connection/Internal/Http2ConnectionProcessor.php(108): Amp\Http\Client\Connection\Internal\Http2ConnectionProcessor->close()
#6 [internal function]: Amp\Http\Client\Connection\Internal\Http2ConnectionProcessor->__destruct()
#7 {main}
  thrown in vendor/amphp/pipeline/src/Internal/QueueState.php on line 349

[trowski]

Are you using the latest beta of http-client? I thought I squashed that in amphp/http-client@9cae0bf. The mprotect failure is suspect though – perhaps you have too many fibers at once? Cannot allocate memory (12) corresponds to ENOMEM, indicating your system ran out of memory.

[bileslav]

amphp/http-client@9cae0bf

If this is a fix for that problem, then the fact is that I didn't have it. The last published release (5.0.0 Beta 3) happened earlier (Jun 17), and it was installed. Regarding the number of fibers or memory exhaustion, if one of these happened and it's not because of the absence of this fix, then the problem is still in the library, because I don't run into limits for sure.

[trowski]

Sorry, I had looked at the date for the latest beta of this library, not http-client. 😊

For now please try v5.x-dev in composer for http-client. You'll need that for @kelunik's change as well.