libmicrohttpd

commit 3eb2fb6dfb302a7ffbc74174d37fa047e3a94036
parent c8b7b8fed96c513c82b2da9037206daccf3eb492
Author: Evgeny Grin (Karlson2k) <k2k@narod.ru>
Date:   Wed,  8 Jun 2022 20:35:29 +0300

Fixed very rare data races when closing upgraded connection

Diffstat:
M
src/microhttpd/daemon.c
 | 
38
++++++++++++++++++++++++++++++++++++++
M
src/microhttpd/internal.h
 | 
13
+++++++++++++
M
src/microhttpd/response.c
 | 
7
++++---

3 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c
@@ -3270,6 +3270,44 @@ MHD_resume_connection (struct MHD_Connection *connection)
 }
 
 
+#ifdef UPGRADE_SUPPORT
+/**
+ * Mark upgraded connection as closed by application.
+ *
+ * The @a connection pointer must not be used after call of this function
+ * as it may be freed in other thread immediately.
+ * @param connection the upgraded connection to mark as closed by application
+ */
+void
+upgraded_connection_mark_app_closed_ (struct MHD_Connection *connection)
+{
+  /* Cache 'daemon' here to avoid data races */
+  struct MHD_Daemon *const daemon = connection->daemon;
+#if defined(MHD_USE_THREADS)
+  mhd_assert (NULL == daemon->worker_pool);
+#endif /* MHD_USE_THREADS */
+  mhd_assert (NULL != connection->urh);
+  mhd_assert (0 != (daemon->options & MHD_TEST_ALLOW_SUSPEND_RESUME));
+
+  MHD_mutex_lock_chk_ (&daemon->cleanup_connection_mutex);
+  connection->urh->was_closed = true;
+  connection->resuming = true;
+  daemon->resuming = true;
+  MHD_mutex_unlock_chk_ (&daemon->cleanup_connection_mutex);
+  if ( (MHD_ITC_IS_VALID_ (daemon->itc)) &&
+       (! MHD_itc_activate_ (daemon->itc, "r")) )
+  {
+#ifdef HAVE_MESSAGES
+    MHD_DLOG (daemon,
+              _ ("Failed to signal resume via " \
+                 "inter-thread communication channel.\n"));
+#endif
+  }
+}
+
+
+#endif /* UPGRADE_SUPPORT */
+
 /**
  * Run through the suspended connections and move any that are no
  * longer suspended back to the active state.
diff --git a/src/microhttpd/internal.h b/src/microhttpd/internal.h
@@ -2482,4 +2482,17 @@ MHD_get_master (struct MHD_Daemon *const daemon)
 }
 
 
+#ifdef UPGRADE_SUPPORT
+/**
+ * Mark upgraded connection as closed by application.
+ *
+ * The @a connection pointer must not be used after call of this function
+ * as it may be freed in other thread immediately.
+ * @param connection the upgraded connection to mark as closed by application
+ */
+void
+upgraded_connection_mark_app_closed_ (struct MHD_Connection *connection);
+#endif /* UPGRADE_SUPPORT */
+
+
 #endif
diff --git a/src/microhttpd/response.c b/src/microhttpd/response.c
@@ -1773,11 +1773,12 @@ MHD_upgrade_action (struct MHD_UpgradeResponseHandle *urh,
     }
 #endif /* HTTPS_SUPPORT */
     mhd_assert (MHD_CONNECTION_UPGRADE == connection->state);
-    urh->was_closed = true;
-    /* As soon as connection will be marked with BOTH
+    /* The next function will mark the connection as closed by application
+     * by setting 'urh->was_closed'.
+     * As soon as connection will be marked with BOTH
      * 'urh->was_closed' AND 'urh->clean_ready', it will
      * be moved to cleanup list by MHD_resume_connection(). */
-    MHD_resume_connection (connection);
+    upgraded_connection_mark_app_closed_ (connection);
     return MHD_YES;
   case MHD_UPGRADE_ACTION_CORK_ON:
     /* Unportable API. TODO: replace with portable action. */