Issue with Apache Bench and deferred end

[elan]

Forgive me if I'm misunderstanding how Loop::defer works, but I've observed an interesting side effect when deferring the response's end. I started by doing the standard ab -n 1000 http://localhost:3000/ and was marveling at the speed of this library.

Because of how it's being integrated (need to process requests async), I figured I'd made use of Loop::defer, but even when used in the simplest way, it causes ab -n 1 http://localhost:3000/ to hang until timeout (~10 seconds).

This is the change I made to HelloWorld.cpp to cause the issue. Note that curl http://localhost:3000/ still works fine, probably because it's only ever expecting to make a single request.

-          res->end("Hello world!");
+          res->onAborted([]() {});
+          uWS::Loop::get()->defer([=]() {
+            res->end("Hello world!");
+          });

Is there something I'm missing about how to defer the end?

[elan]

I should add I've been able to reproduce this behavior change on both macOS and Linux.

[ghost]

You need to set a flag in onAborted handler and check so this flag isn't set before calling end. Defer queues a lambda for execution in the future when the very response might have been aborted. That's why onAborted must be used, but you do nothing in it.

[elan]

Sorry, I should have been more clear there; onAborted is not being called in this example, so setting a flag wouldn't have any effect (I do set a flag in my code to handle this case). There's something which goes wrong after calling res->end for a request which has not been aborted.

[ghost]

Ah okay then you've found a bug where the Connection: close behavior is not working like it should and it makes sense.

Btw, you probably want to benchmark with wrk instead of ab. Ab is ancient and uses Http 1.0. That's why it triggers the bug.

[elan]

Thanks for the wrk recommendation, ApacheBench was more muscle memory to me than anything else 😅 And you're right, wrk does not trigger the issue.

The interesting part to me was that as long as I called res->end right away instead of after a Loop::defer, ApacheBench worked too. I stepped through the internalEnd code in both cases, and it seemed to be doing the same thing, hence why I figured it might be a timing thing.

uWebSockets is really quite awesome, I've been enjoying integrating it!

[uNetworkingAB]

How are you testing now that ab is unsupported? You just set connection: close?

[uNetworkingAB]

Here if cork it's works without corking no.

It's the opposite. I have a fix here - it's broken at 3 places:

• When res->cork returns it should shutdown if connectionClose (this complements the automatic corking when not deferred)
• If not corked and done with chunked response it should shutdown if connectionClose regardless from where
• If not corked and done with regular response it should shutdown if connectionClose regardless from where <- this fixes this report

Essentially, the Connection: close / HTTP1.0 closing is sloppy and not very strict right now.

[uNetworkingAB]

Fixed in latest commit