pocoproject / poco

The POCO C++ Libraries are powerful cross-platform C++ libraries for building network- and internet-based applications that run on desktop, server, mobile, IoT, and embedded systems.
https://pocoproject.org
Other
8.42k stars 2.17k forks source link

Freezing requests in HTTPServer with a low max threads number #2252

Closed roman-kruglov closed 2 years ago

roman-kruglov commented 6 years ago

Hello there, this is probably rather a question about architecture understanding, but I'll fill in the predefined template below anyway.

In short: we need to use HTTPServer with a low number of worker threads, like 4-8 or similar. There is a simple test page opened in a browser which makes requests with JS using Ajax directly to the server app.

Expected behavior

requests pass

Actual behavior

Requests sometimes freeze for a long time (around one or several minutes, sometimes seems that forever). The lower the max threads number - the easier to reproduce. Also if I make a new request from another tab, that immediately defreezes all the requests made from the test page in the prev tab. Also seems to happen more often after refreshing the test page which makes requests. Interestingly enough, it didn't seem to reproduce noticeably while testing with JMeter, only a browser. We tried different parameters like turning off keep-alives, but that doesn't change anything. Also it seems to reproduce in any modern browser (tried Chrome, Firefox, IE and even some mobile) so probably not tied to that.

Steps to reproduce the problem

just run any HTTPServer example with the max threads set to 1 and make requests from a browser.

POCO version

1.9.0

Compiler and version

MSVC 2017 Community 15.6.4

Operating system and version

Windows 10

Other relevant information

We are trying to use Poco::Net::HTTPServer framework in our application and encountered some strange behavior. I want to understand the reasons behind it better - and maybe make some alterations accordingly. Though our app is CPU-bound kinda, we have a restriction on the maximum number of worker threads we can use in it, because our application uses several third party libraries, which are not very thread-friendly and we end up allocating some big and memory consuming objects for each thread. In case of many threads our app can consume quite a lot of memory. So we tried to restrict the maximum number of threads to something like the number of CPU cores * 2. Around 4-8 usually.

So I wanted to better understand how the HTTPServer works - is it supposed to be used with such a low number of threads? If it allocates a single thread for a connection, could it be that the browser was making several and sending requests with each of them? But why would a single thread just be idle if the keep-alive is turned off and there are more requests / connections to process? Also does the maximum number of threads effectively limit the maximum number of concurrent connections to the server?

Maybe you have other recommendations for our and similar cases (need to keep low threads count)? I was also looking at the parallel reactor, but it seems harder to adapt our legacy code to. Maybe it is better to use unlimited HTTPServer threads and just schedule and process the requests in a limited number of our worker threads?

Also I have some more data like WireShark sniffs and some debugging attempts of Poco code which I can share later, but I'm starting to think more that it's just how it's supposed to work.

aleks-f commented 6 years ago

The way it works is that every request is processed in it's own thread, taken from the default (unless you have provided your own) thread pool. So, threads are not spawned to handle requests, but taken from already available pool. A small number of threads should not create problems.

Generally speaking, anything that happens in the request handler should be as minimal as possible in terms of timing - if you have some heavy work and the client can handle not to receive the results immediately, get what you need in handler and do the work in another thread. Anything in the server that should live across multiple requests should be created at the request handler factory level and access to it from other threads synchronized accordingly.

It is hard to tell more without seeing exactly what you are doing. Now, please don't dump your whole app on us, but if you can make a SSCCE reproducing the problem, we will look into it to make sure it is not poco-related (often, the root cause may be discovered while trying to produce a simple reproduction of the problem). I'd also suggest to look into how are you dealing with the data shared between threads - race conditions or some logic that manifests as you described (new request thawing previously frozen ones).

roman-kruglov commented 6 years ago

Here is the complete minimal example to reproduce this behavior, basically just a modified example from the repo with the max threads num set to 1. Keep alives are turned off to exclude possibility of an open hanging connection which blocks the single thread until the keep alive timeout is reached, but it doesn't change anything, reproduces with or without it. All other params we tried didn't matter as well. As I said previously, It reproduces on a higher number of threads too, just less frequently and was harder to notice until we isolated it. The env is the same as described in my first post, Win10 (or 7, doesn't change), MSVC 2017, Poco 1.9.0 (actually it reproduced with an older Poco version too, that was the reason we tried upgrading to the latest).

#include <iostream>

#include "Poco/Net/HTTPRequestHandlerFactory.h"
#include "Poco/Net/HTTPServerRequest.h"
#include "Poco/Net/HTTPServer.h"
#include "Poco/Net/HTTPServerParams.h"
#include "Poco/Net/HTTPRequestHandler.h"
#include "Poco/Net/HTTPServerResponse.h"
#include "Poco/StreamCopier.h"

using namespace std;
using Poco::Net::HTTPServer;
using Poco::Net::HTTPServerParams;
using Poco::Net::ServerSocket;
using Poco::Net::HTTPRequestHandlerFactory;
using Poco::ThreadPool;

class DataHandler : public Poco::Net::HTTPRequestHandler
{
public:
    void handleRequest(Poco::Net::HTTPServerRequest& request,
        Poco::Net::HTTPServerResponse& response)
    {
        response.setContentType("text/html");
        std::ostream& ostr = response.send();
        ostr << "HELLO!";
    }
};

class MyRequestHandlerFactory : public
    Poco::Net::HTTPRequestHandlerFactory
{
public:
    MyRequestHandlerFactory()
    {
    }
    Poco::Net::HTTPRequestHandler* createRequestHandler(
        const Poco::Net::HTTPServerRequest& request)
    {
        return new DataHandler();
    }
};

int main()
{
    Poco::UInt16 port = 2880;
    HTTPServerParams* pParams = new HTTPServerParams;
    pParams->setKeepAlive(false);
    pParams->setMaxThreads(1);
    ServerSocket svs(port);

    HTTPServer srv(new MyRequestHandlerFactory(), svs, pParams);
    srv.start();

    std::cout << "Server Started" << std::endl;
    std::cin.get();
    return 0;
}

also here is a test page to alleviate reproducing. It sends a simple Ajax GET request on click on page's body. The very important note is that it reproduces on the first request from the browser to the server. When that first one de-freezes all next requests will be successful. After a page refresh the first click sends and reproduces again. Approximate reproducibility is subjectively around 90% of attempts, not solid 100%:

<script
  src="https://code.jquery.com/jquery-3.3.1.min.js"
  integrity="sha256-FgpCb/KJQlLNfOu91ta32o/NMZxltwRo8QtmkMRdAu8="
  crossorigin="anonymous"></script>

<script>
$(function() {
    $('body').click(function() {
        (function(ind) {
            $.ajax({
                url: "http://localhost:2880/",
                method: "GET",
                success: function(data) {
                    console.log(data);
                }
            });
        })(1);
    })
});

</script>

Also a note about the code which hangs - in this example (and in our app's code too) it doesn't even get to the handler's code. If you set a breakpoint and manage to get a frozen (pending) request, it won't hit the break point until it de-freezes after around a minute or two or if you try to send a separate manual request from another browser's tab URL field. In this case you will get the first made request at the break point followed by all next previously pending requests.

roman-kruglov commented 6 years ago

Also maybe you could answer several questions while you're here. Several from the first post were answered, is my understanding of the answer correct?

  1. Is it supposed to be used with such a low number of threads? - from your answer it seems it could be used like that, should work ok.
  2. If it allocates a single thread for a connection, ... - you answered that it's a thread per request, so each requested is processed like this: request comes, Poco takes a thread from the threadpool, the thread processes the request, Poco returns this thread to the pool, right?
  3. But why would a single thread just be idle if the keep-alive is turned off and there are more requests / connections to process? - from your answer I assume that it shouldn't ever happen.
  4. Does the maximum number of threads effectively limit the maximum number of concurrent connections to the server? - no answer or I didn't get it from the context. This one would be important to know if you can

And only one new - how can we use separate threads for processing? Should we just put the.. Poco::Net::HTTPServerRequests to a queue? It's kinda hard because they are abstract and have a separate implementation and we don't have a shared ptr to them, right? Also I suppose the request is finished after the processing thread leaves the handler body. If we delegate to another thread, this one (Poco thread) should just.. wait to not leave the handler's body? Seems like no point in doing that.

Really appreciate your answers & help, thanks

roman-kruglov commented 6 years ago

Guys? After your initial explanation this does seem like a bug. Can you please mark it as a bug?

obiltschnig commented 6 years ago

I can confirm that it's a known issue with only one thread.

Correction: The issue I remember was setting the max queued size to 1, which does not work.

As for more threads, I have not seen this happening. It could be an issue with the default ThreadPool becoming exhausted, although I doubt it.

However, I have to look into this. May take some time until I get to that, though.

One thing, though: your handleRequest() method should either set the Content-Length or enable chunked transfer encoding.

obiltschnig commented 6 years ago

At least on my Mac, I cannot reproduce this issue with your sample.

roman-kruglov commented 6 years ago

Reproducible under Win 7/10 on our side, not sure about Macs. We'll check this min example on Linux (Ubuntu and CentOS are our targets). But in general in our app code this does seem to happen in linux too. Just gotta try the isolated one to exclude our code..

Also yeah, we use chunked encoding in our app, we probably just excluded this line too. This will probably not influence the bug anyways.

obiltschnig commented 6 years ago

Tried now under Win7 With VS2017 and still cannot reproduce.

obiltschnig commented 6 years ago

I'm really suspecting something related to your environment, as I've been using the web server quite extensively over the years and never had such an issue.

roman-kruglov commented 6 years ago

Maybe it's somehow related to our compilation flags or some cmake options.. I'll try the min example on Linux and make sure it uses only defaults. But probably in a week or so - we are having a release and all that fuss. I'll let you know about any results, thank you for your time and effort.

bellerofonte commented 5 years ago

Reproducing same issue on Linux Mint 19, Poco version 1.9.0. Using HTTPServer with default thread pool with max threads = 8 (cpu count = 4) If I interrupt execution when a HTTP request freezes, I have 8 threads paused at following instructions:

0x7fa010df1f7e  <+  814>        b8 ca 00 00 00              mov    $0xca,%eax
0x7fa010df1f83  <+  819>        0f 05                       syscall
-->  0x7fa010df1f85  <+  821>        48 3d 00 f0 ff ff           cmp    $0xfffffffffffff000,%rax
0x7fa010df1f8b  <+  827>        0f 87 99 01 00 00           ja     0x7fa010df212a <__pthread_cond_timedwait+1242>
        207 [1] in ../sysdeps/unix/sysv/linux/futex-internal.h
0x7fa010df1f91  <+  833>        8b 7c 24 50                 mov    0x50(%rsp),%edi

In such cases createRequestHandler method never gets called May be this could help in solving this issue.

infernalboy commented 4 years ago

I forgot to pass the socket to the constructor. It took a year to understand this ... After deleting the first line, the problem was not repeated.

 m_serverSoket = std::make_shared<Poco::Net::ServerSocket>(m_port); // <- This is a mistake of epic stupidity
 m_httpServer =std::make_shared<Poco::Net::HTTPServer>(new RequestHandlerFactory, m_port, m_params);

(Sorry for my English, I used Google)

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 365 days with no activity.