[libmicrohttpd] more on MHD_USE_THREAD_PER_CONNECTION and races

[Eivind Sarto]

I have some additional info on races in the library when using 
MHD_USE_THREAD_PER_CONNECTION.

It looks like MHD_handle_connection() races against MHD_cleanup_connections() 
after MHD_connection_close() gets called.
The problem is with the main loop in MHD_handle_connection():

MHD_handle_connection(void *data)
{
  struct MHD_Connection *con = data;

  while ((!con->daemon->shutdown) && (con->socket_fd != -1)) {
       /* whenever MHD_connection_close() is called from within this loop (or 
from any place within
        * the state machine, there is a race with MHD_cleanup_connections().
        */
  }
}

The problem is that the while-loop will come back and reference the connection 
structure, but after a MHD_connection_close() the
MHD daemon may have freed the connection structure.


Maybe this is a good time to also fix a big performance issue with using MHD 
with a lot of connections.
MHD_cleanup_connections() scans all the connections after a new connection has 
been accepted.
If you have 1000s of connections that are keep-alive, you will be scanning 
1000s of connections to find that one connection
that have been closed.

Maybe it would be a better idea to implement an "active" connection list and a 
"closed" connection list (both doubly linked lists).
Protect them with a mutex.  After a connection has been closed and  the 
connection thread is completely done referencing the structute
(like at the end of MHD_handle_connection), it grabs the lock and moves the 
connection structure from the "active" to the "closed" connection list. 
The MHD_cleanup_connections() also grabs the lock and only cleans up 
connections on the "closed" list.
No race, and much less overhead with lots of connections.

What do you think?

-eivind