Re: [libmicrohttpd] Suspend/resume with single thread and external epoll not sending response

[Robert D Kocisko]

Hi Christian,

Thank you!  Very cool to see the example working.  Funny, I
implemented MHD_get_timeout in the main loop in my project but in all
my tests it always returned MHD_NO, so I assumed it was not applicable
to my use case and didn't include it in the example I wrote.  But I
didn't make the connection that it *also* had to be called after
MHD_resume_connection until your response here.  Would you consider
the attached patch?   It adds one line to the docs that I'm quite sure
would have pointed me in the right direction had it been there before.

Thanks!
Bob

On Sun, Mar 11, 2018 at 1:11 PM, Christian Grothoff <address@hidden> wrote:
>
> Dear Bob,
>
> I've analyzed your code, and the issue is on your end: you simply didn't
> set the timeout correctly. When using an external event loop, it is
> mandatory that you ask MHD for the timeout using MHD_get_timeout() and
> use that with select/poll/epoll.  Then, you must call MHD_run() once
> whenever epoll() returns (including timeouts!).
>
> In your case, MHD would have given you a timeout of 0, but you used
> infinity instead, with predictable results...
>
> I've attached a corrected version of the code.
>
> Happy hacking!
>
> Christian
>
> On 03/02/2018 08:26 PM, Robert D Kocisko wrote:
> > First, thanks for your amazing work on MHD!
> >
> > This question is a near duplicate of the 2014 message thread from Tom
> > Cornell entitled "Trouble getting a response sent from a separate worker
> > thread (with external select)".  However, I am not using separate worker
> > threads--everything is in one thread and so I don't think the
> > recommendations found in that thread apply to my scenario.
> >
> > Basically after receiving a request from an HTTP client, I want to be
> > able to do some asynchronous 'work' which is really just waiting on
> > another process such as a database engine to calculate and return the
> > result which, when complete, I will forward back to the client.  This is
> > all done in one thread using epoll, so I don't want any blocking and I
> > don't want any busy loops.  MHD's external epoll support combined with
> > suspend/resume fits into this architecture perfectly, but there's a
> > problem: after resuming the connection and queueing the data, the
> > headers are sent to the client immediately, but the body of the response
> > does not get sent until another client request arrives.
> >
> > Anyway, to make this all concrete, I've put together a small working
> > example (below) which shows the problem.  This is built against the
> > latest dev rev (7f1dbb2) on elementaryOS (which is Ubuntu 16.04).  Every
> > time a request comes in it suspends the connection and starts a 1 second
> > timer which, when it expires, resumes the connection.  When the
> > connection is resumed the response is queued (simply echos the request
> > url).  I realize this example leaks timer fds and doesn't clean up
> > properly but it successfully demonstrates the problem.
> >
> > I have experimented with calling MHD_run() twice after
> > MHD_resume_connection() rather than the once required by the docs, and
> > that does seem to work, but that seems extremely hacky and I'm not sure
> > if twice is enough (why twice and not three times?).  I've skimmed the
> > source code looking for obvious answers but none are readily apparent to me.
> >
> > At this point I'm pretty sure that this is a bug with MHD but am I
> > missing something?
> >
> > Thanks!
> > Bob Kocisko
> >
> > -------------------------
> >
> > #include "platform.h"
> > #include <microhttpd.h>
> > #include <sys/epoll.h>
> > #include <sys/timerfd.h>
> >
> > #define TIMEOUT_INFINITE -1
> >
> > struct Request {
> >   struct MHD_Connection *connection;
> >   int timerfd;
> > };
> >
> > int epfd;
> > struct epoll_event evt;
> >
> > static int
> > ahc_echo (void *cls,
> >           struct MHD_Connection *connection,
> >           const char *url,
> >           const char *method,
> >           const char *version,
> >           const char *upload_data, size_t *upload_data_size, void **ptr)
> > {
> >   struct MHD_Response *response;
> >   int ret;
> >   struct Request* req;
> >   struct itimerspec ts;
> >   (void)url;               /* Unused. Silent compiler warning. */
> >   (void)version;           /* Unused. Silent compiler warning. */
> >   (void)upload_data;       /* Unused. Silent compiler warning. */
> >   (void)upload_data_size;  /* Unused. Silent compiler warning. */
> >
> >   req = *ptr;
> >   if (!req)
> >   {
> >
> >     req = malloc(sizeof(struct Request));
> >     req->connection = connection;
> >     req->timerfd = 0;
> >     *ptr = req;
> >     return MHD_YES;
> >   }
> >
> >   if (req->timerfd)
> >   {
> >     // send response (echo request url)
> >     response = MHD_create_response_from_buffer (strlen (url),
> >                                                 (void *) url,
> >                                                 MHD_RESPMEM_MUST_COPY);
> >     ret = MHD_queue_response (connection, MHD_HTTP_OK, response);
> >     MHD_destroy_response (response);
> >     return ret;
> >   }
> >   else
> >   {
> >     // create timer and suspend connection
> >     req->timerfd = timerfd_create(CLOCK_MONOTONIC, TFD_NONBLOCK);
> >     if (-1 == req->timerfd)
> >     {
> >       printf("timerfd_create: %s", strerror(errno));
> >       return MHD_NO;
> >     }
> >     evt.events = EPOLLIN;
> >     evt.data.ptr = req;
> >     if (-1 == epoll_ctl(epfd, EPOLL_CTL_ADD, req->timerfd, &evt))
> >     {
> >       printf("epoll_ctl: %s", strerror(errno));
> >       return MHD_NO;
> >     }
> >     ts.it_value.tv_sec = 1;
> >     ts.it_value.tv_nsec = 0;
> >     ts.it_interval.tv_sec = 0;
> >     ts.it_interval.tv_nsec = 0;
> >     if (-1 == timerfd_settime(req->timerfd, 0, &ts, NULL))
> >     {
> >       printf("timerfd_settime: %s", strerror(errno));
> >       return MHD_NO;
> >     }
> >
> >     MHD_suspend_connection(connection);
> >     return MHD_YES;
> >   }
> > }
> >
> > static int
> > connection_done(struct MHD_Connection *connection,
> >                 void **con_cls,
> >                 enum MHD_RequestTerminationCode toe)
> > {
> >   free(*con_cls);
> > }
> >
> > int
> > main (int argc, char *const *argv)
> > {
> >   struct MHD_Daemon *d;
> >   const union MHD_DaemonInfo * info;
> >   int current_event_count;
> >   struct epoll_event events_list[1];
> >   struct Request *req;
> >   uint64_t timer_expirations;
> >
> >   if (argc != 2)
> >     {
> >       printf ("%s PORT\n", argv[0]);
> >       return 1;
> >     }
> >   d = MHD_start_daemon (MHD_USE_EPOLL | MHD_ALLOW_SUSPEND_RESUME,
> >                         atoi (argv[1]),
> >                         NULL, NULL, &ahc_echo, NULL,
> >                         MHD_OPTION_NOTIFY_COMPLETED, &connection_done, NULL,
> > MHD_OPTION_END);
> >   if (d == NULL)
> >     return 1;
> >
> >   info = MHD_get_daemon_info(d, MHD_DAEMON_INFO_EPOLL_FD);
> >   if (info == NULL)
> >     return 1;
> >
> >   epfd = epoll_create1(EPOLL_CLOEXEC);
> >   if (-1 == epfd)
> >     return 1;
> >
> >   evt.events = EPOLLIN;
> >   evt.data.ptr = NULL;
> >   if (-1 == epoll_ctl(epfd, EPOLL_CTL_ADD, info->epoll_fd, &evt))
> >     return 1;
> >
> >   while (1)
> >   {
> >     current_event_count = epoll_wait(epfd, events_list, 1,
> > TIMEOUT_INFINITE);
> >
> >     if (1 == current_event_count)
> >     {
> >       if (events_list[0].data.ptr)
> >       {
> >         // A timer has timed out
> >         req = events_list[0].data.ptr;
> >         // read from the fd so the system knows we heard the notice
> >         if (-1 == read(req->timerfd, &timer_expirations,
> > sizeof(timer_expirations)))
> >         {
> >           return 1;
> >         }
> >         // Now resume the connection
> >         MHD_resume_connection(req->connection);
> >         if (!MHD_run(d))
> >           return 1;
> >       }
> >       else
> >       {
> >         // MHD is ready
> >         if (!MHD_run(d))
> >           return 1;
> >       }
> >     }
> >     else if (0 == current_event_count)
> >     {
> >       // no events: continue
> >     }
> >     else
> >     {
> >       // error
> >       return 1;
> >     }
> >   }
> >
> >   return 0;
> > }
> >
> >

doc.patch
Description: Text Data