[PATCH] Fix delayed shutdown when smsbox vanishes with high

Hi Stipe,

Thanks for this patch.  By the way David Schneider sent a patch to the devel
group on Tue, 31 Mar 2015 11:49:47 +0200.
When you are committing your code, please consider reviewing his patch as
well and commit it if you think it's necessary.

If developer's patches are ignored they will not take the time to send them
to Kannel Devel and Kannel will not improve.

If this patch has already been reviewed then please ignore this email.

Thanks
Hillel

Message: 1
Date: Tue, 31 Mar 2015 11:49:47 +0200
From: David Schneider <[email protected]>
To: <[email protected]>
Subject: concurrency issue with http_close_all_ports
Message-ID: <[email protected]>
Content-Type: text/plain; charset="utf-8"; format=flowed

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>");