[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [libmicrohttpd] 0.9.71+ Connection Idle and Reuse Issue when Suspend
From: |
Christian Grothoff |
Subject: |
Re: [libmicrohttpd] 0.9.71+ Connection Idle and Reuse Issue when Suspending and Resuming Connections |
Date: |
Wed, 6 Jan 2021 21:25:53 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 |
Hi Damon,
The problem is that your logic queues a response during the *first*
callback to 'access_handler', and your 'count' protection there is not
working as intended by you.
You need to be a bit careful with the API, the
MHD_OPTION_NOTIF_CONNECTIONCATION is per socket, while what you need for
_that_ counter is MHD_URI_LOG_CALLBACK and MHD_OPTION_NOTIFY_COMPLETED.
We _know_ that some of our names are confusing for historic reasons, and
I hope that if we ever find funding for a "MHD v2.0" we can make the API
_much_ more intuitive.
Anyway, I've attached a revision of your code that fixes the bad
behavior. Basically: make sure not to queue a response "early" (as per
line connection.c:4014), and MHD will be happy to do pipelining. (This
is not really a regression, more of a bugfix, as before this could
result in really bad unwarranted behaviors --- and we always said that
you should ONLY queue 'early' *if* the client requested 100 CONTINUE and
you want to _abort_ the upload. And in that case, we _must_ force a
"Connection: close").
Happy hacking!
Christian
On 1/5/21 6:32 PM, Earp, Damon N. (GSFC-619.0)[SCIENCE SYSTEMS AND
APPLICATIONS INC] via libmicrohttpd wrote:
> Hey all,
> We're witnessing an increase in one time use connections when testing against
> 0.9.71 and 0.9.72. It appears to be related to suspending a connection
> immediately then resuming it in a different thread. We are using Linux
> (Debian 9.13 amd64). I wrote up a minimal test program and attempted to test
> connection reuse and the connection idle timeout. Below are the results of
> each test when running with the old and new versions of the library and the
> test source code.
>
> For the tests:
> Version 0.9.62 is provided via aptitude and installed under /usr/lib/.
> Version 0.9.71 is custom compiled without providing any arguments to
> `./configure` and is located in /usr/local/lib.
>
> Thanks for all your hard work. Hopefully there is a small fix for this.
> Damon Earp
>
> --------------------------------------------------------------------------------
> TEST 1 - Connection Reuse
> Client Command: `$ curl -v localhost:8080{,,,,,,,,,,,,,,}`
>
> libmicrohttpd 0.9.62
> ```
> $ ./idle
> close after 0.0046 seconds, completed 15 requests
> ```
>
> libmicrohttpd 0.9.71
> ```
> $ LD_LIBRARY_PATH=/usr/local/lib ./idle
> close after 0.0004 seconds, completed 1 requests
> close after 0.0002 seconds, completed 1 requests
> close after 0.0002 seconds, completed 1 requests
> close after 0.0002 seconds, completed 1 requests
> close after 0.0002 seconds, completed 1 requests
> close after 0.0002 seconds, completed 1 requests
> close after 0.0002 seconds, completed 1 requests
> close after 0.0002 seconds, completed 1 requests
> close after 0.0002 seconds, completed 1 requests
> close after 0.0002 seconds, completed 1 requests
> close after 0.0002 seconds, completed 1 requests
> close after 0.0002 seconds, completed 1 requests
> close after 0.0002 seconds, completed 1 requests
> close after 0.0001 seconds, completed 1 requests
> close after 0.0002 seconds, completed 1 requests
> ```
>
> --------------------------------------------------------------------------------
> TEST 2 - Idle Connection Timeout of 5 Seconds.
> Client Command: `$ perl -e '$|=1;print "GET /index.html
> HTTP/1.1\r\n\r\n";while(<>){}' | nc localhost 8080`
>
> libmicrohttpd 0.9.62
> ```
> $ ./idle
> close after 5.6223 seconds, completed 1 requests
> ```
>
> libmicrohttpd 0.9.71
> ```
> $ LD_LIBRARY_PATH=/usr/local/lib ./idle
> close after 0.0027 seconds, completed 1 requests
> ```
>
> --------------------------------------------------------------------------------
> Test Program
> Build command: `gcc -Wall -Werror -o idle main.c -lmicrohttpd -pthread`
>
> ```
> #include <microhttpd.h>
> #include <pthread.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <arpa/inet.h>
> #include <sys/socket.h>
> #include <time.h>
> #include <unistd.h>
>
> #if MHD_VERSION < 0x00097000
> #define MHD_RESULT_TYPE int
> #else
> #define MHD_RESULT_TYPE enum MHD_Result
> #endif
>
> struct connection {
> struct timespec begin;
> unsigned count;
> };
>
> static void *worker(void *userdata)
> {
> struct MHD_Connection *con = userdata;
> MHD_resume_connection(con);
> return NULL;
> }
>
> static void connection_notify(void* cls, struct MHD_Connection* con,
> void** ctx, enum MHD_ConnectionNotificationCode code)
> {
> if (code == MHD_CONNECTION_NOTIFY_STARTED)
> {
> struct connection *c = calloc(sizeof(*c), 1);
> clock_gettime(CLOCK_MONOTONIC, &c->begin);
> *ctx = c;
> }
> else if (code == MHD_CONNECTION_NOTIFY_CLOSED)
> {
> struct connection *c = *ctx;
> struct timespec now, diff;
> clock_gettime(CLOCK_MONOTONIC, &now);
> diff.tv_sec = now.tv_sec - c->begin.tv_sec;
> if ((diff.tv_nsec = now.tv_nsec - c->begin.tv_nsec) < 0)
> {
> --diff.tv_sec;
> diff.tv_nsec += 1000000000;
> }
> double secs = (diff.tv_sec * 1000000000 + diff.tv_nsec) /
> 1000000000.0;
> printf("close after %.4lf seconds, completed %u requests\n", secs,
> c->count);
> free(c);
> }
> }
>
> static MHD_RESULT_TYPE access_handler(void *cls, struct MHD_Connection *con,
> const char *url, const char *method, const char *version,
> const char *upload_data, size_t *upload_data_size, void **con_cls)
> {
> /*
> * hackish trick to detect if this is the first or second time the
> handler
> * has been called for a request.
> */
> uintptr_t *count = (void*) con_cls;
> if ((*count)++ == 0)
> {
> ((struct connection*) MHD_get_connection_info(con,
> MHD_CONNECTION_INFO_SOCKET_CONTEXT)->socket_context)->count++;
> MHD_suspend_connection(con);
> pthread_t t;
> pthread_create(&t, NULL, &worker, con);
> pthread_detach(t);
> }
> else
> {
> struct MHD_Response *resp = MHD_create_response_from_buffer(0, NULL,
> MHD_RESPMEM_PERSISTENT);
> MHD_queue_response(con, 200, resp);
> MHD_destroy_response(resp);
> }
> return MHD_YES;
> }
>
> int main(int argc, const char *argv[])
> {
> int flags = MHD_USE_EPOLL_INTERNAL_THREAD | MHD_USE_ITC |
> MHD_ALLOW_SUSPEND_RESUME;
> struct MHD_Daemon *d = MHD_start_daemon(flags, 8080, NULL, NULL,
> &access_handler, NULL,
> MHD_OPTION_NOTIFY_CONNECTION, &connection_notify, NULL,
> MHD_OPTION_CONNECTION_TIMEOUT, 5u,
> MHD_OPTION_END);
> pause();
> MHD_stop_daemon(d);
> return 0;
> }
> ```
>
test.c
Description: Text Data
signature.asc
Description: OpenPGP digital signature