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;
> }
> ```
>
#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;
};
struct request
{
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 void *
started_callback (void*cls, const char *uri,
struct MHD_Connection*con)
{
struct request *c = calloc (sizeof(*c), 1);
clock_gettime (CLOCK_MONOTONIC, &c->begin);
return c;
}
static void
completed_callback (void *cls,
struct MHD_Connection *connection,
void **con_cls,
enum MHD_RequestTerminationCode toe)
{
struct request *c = *con_cls;
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 ("request done after %.4lf seconds\n", secs);
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.
*/
struct request *ic = (struct request*) *con_cls;
if (0 == ic->count)
{
ic->count++;
return MHD_YES;
}
if (1 == ic->count)
{
ic->count++;
MHD_suspend_connection (con);
pthread_t t;
pthread_create (&t, NULL, &worker, con);
pthread_detach (t);
return MHD_YES;
}
{
enum MHD_Result res;
struct MHD_Response *resp = MHD_create_response_from_buffer (0, NULL,
MHD_RESPMEM_PERSISTENT);
res = MHD_queue_response (con, 200, resp);
MHD_destroy_response (resp);
((struct connection*) MHD_get_connection_info(con, MHD_CONNECTION_INFO_SOCKET_CONTEXT)->socket_context)->count++;
return res;
}
}
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_COMPLETED,
&completed_callback, NULL,
MHD_OPTION_URI_LOG_CALLBACK,
&started_callback, NULL,
MHD_OPTION_NOTIFY_CONNECTION, &connection_notify, NULL,
MHD_OPTION_CONNECTION_TIMEOUT, 5u,
MHD_OPTION_END);
pause ();
MHD_stop_daemon (d);
return 0;
}
signature.asc
Description: OpenPGP digital signature
