Hello

I found a concurrency issue with the latest http server implementation.

I'm running test_http_server with multiple threads (-t 5). If I send a request which is delayed by a sleep and in the mean time I'm calling "http://localhost:8080/quit"; then the server crashes during http_send_reply call due to already freed client->conn.

Index: test/test_http_server.c
===================================================================
--- test/test_http_server.c    (Revision 5141)
+++ test/test_http_server.c    (working copy)
@@ -237,6 +237,9 @@
         if (extra_headers != NULL)
             http_header_combine(resph, extra_headers);

+        if (run)
+            gwthread_sleep(5);
+
         /* return response to client */
         http_send_reply(client, status, resph, reply_body);


testing with:
test/test_http_server -t 5 &
curl "http://localhost:8080"; &
sleep 1
curl "http://localhost:8080/quit";

I found a possible solution in http.c: port_remove

Index: gwlib/http.c
===================================================================
--- gwlib/http.c    (Revision 5141)
+++ gwlib/http.c    (working copy)
@@ -2097,8 +2097,14 @@
      */
     gwlist_lock(active_connections);
     l = gwlist_search_all(active_connections, &port, port_match);
-    while(l != NULL && (client = gwlist_extract_first(l)) != NULL)
+    while(l != NULL && (client = gwlist_extract_first(l)) != NULL) {
         conn_unregister(client->conn);
+        while (client->state == request_is_being_handled) {
+            gwthread_sleep(0.1);
+        }
+    }
     gwlist_unlock(active_connections);
     gwlist_destroy(l, NULL);
while((client = gwlist_search(active_connections, &port, port_match)) != NULL)


What do you think about that solution?

David

PS: There is a typo in test_http_server help:
Index: test/test_http_server.c
===================================================================
--- test/test_http_server.c    (Revision 5141)
+++ test/test_http_server.c    (working copy)
@@ -288,7 +291,7 @@
     info(0, "    read HTTP headers from file 'filename' and add them to");
     info(0, "    the request for url 'url'");
     info(0, "specific URIs with special functions are:");
-    info(0, "  /quite - shutdown the HTTP server");
+    info(0, "  /quit - shutdown the HTTP server");
     info(0, "  /whitelist - provides the -w whitelist as response");
     info(0, "  /blacklist - provides the -b blacklist as response");
info(0, " /save - save a HTTP POST request body to a file /tmp/body.<pid>.<n>");


Reply via email to