Hi Damon, You're right, the combination of MHD_NO_LISTEN_SOCKET and MHD internal threads did not work properly. However, the 'race' you are trying to fix doesn't really apply. Also, I think it is a bit safer to test for the existence of the IPC socket before creating the threads in the no-listen-socket case (as otherwise we cannot stop the threads). But, that should be a very, very minor detail.
After confirming that it seems to fix the test you provided, I have now
pushed a patch to Git as revision efd1dd05..1d9f940d. The fix is a
minor variation on your suggestion, so thanks for the report and diagnosis!
Happy hacking!
Christian
On 9/23/20 11:28 PM, Damon Earp wrote:
> Hoping this is as a reply to my previous email.
>
> I did some digging and below is a patch to daemon.c which allows an
> epoll internal thread to have connections added to it from another
> thread. This patch removes the condition where if MHD_NO_LISTEN_SOCKET
> is present the internal thread isn't started. And it moves setting the
> connection->epoll_state to before calling epoll_ctl avoiding a race
> condition when MHD_add_connection is called from another thread.
>
> This patch is specific to epoll_internal w/o turbo and was done ad hoc
> by an ignorant developer. If another type of setup was used this patch
> would probably cause problems.
>
> Are there other hazards to this? From my gdb tracing it looks like all
> the daemon accesses are synchronized, but I'm interested in getting some
> feedback from someone who actually knows the code and can point me in a
> better direction.
>
> Thanks,
> Damon
>
> --- libmicrohttpd-0.9.71/src/microhttpd/daemon.c 2020-06-16
> 18:38:13.000000000 +0000
> +++ libmicrohttpd-0.9.71-patched/src/microhttpd/daemon.c 2020-09-23
> 21:08:52.000000000 +0000
> @@ -2718,11 +2718,13 @@
>
> event.events = EPOLLIN | EPOLLOUT | EPOLLPRI | EPOLLET;
> event.data.ptr = connection;
> + connection->epoll_state |= MHD_EPOLL_STATE_IN_EPOLL_SET;
> if (0 != epoll_ctl (daemon->epoll_fd,
> EPOLL_CTL_ADD,
> client_socket,
> &event))
> {
> + connection->epoll_state &= ~MHD_EPOLL_STATE_IN_EPOLL_SET;
> eno = errno;
> #ifdef HAVE_MESSAGES
> MHD_DLOG (daemon,
> @@ -6438,8 +6440,7 @@
> }
> #endif /* HTTPS_SUPPORT */
> #if defined(MHD_USE_POSIX_THREADS) || defined(MHD_USE_W32_THREADS)
> - if ( (0 != (*pflags & MHD_USE_INTERNAL_POLLING_THREAD)) &&
> - (0 == (*pflags & MHD_USE_NO_LISTEN_SOCKET)) )
> + if ( (0 != (*pflags & MHD_USE_INTERNAL_POLLING_THREAD)) )
> {
> if (0 == daemon->worker_pool_size)
> {
signature.asc
Description: OpenPGP digital signature
