QUIC REACTOR: Allow ticks to schedule notifications of other threads

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24971)

diff --git a/include/internal/quic_reactor.h b/include/internal/quic_reactor.h
index aa55d378647af3c15d5e0514008e3b6e4c0911af..8d20a073431c632c72c108a7497ce9e338d74f4f 100644 (file)
--- a/include/internal/quic_reactor.h
+++ b/include/internal/quic_reactor.h
@@ -73,6 +73,7 @@
 struct quic_tick_result_st {
     char        net_read_desired;
     char        net_write_desired;
+    char        notify_other_threads;
     OSSL_TIME   tick_deadline;
 };
 
@@ -80,9 +81,10 @@ static ossl_inline ossl_unused void
 ossl_quic_tick_result_merge_into(QUIC_TICK_RESULT *r,
                                  const QUIC_TICK_RESULT *src)
 {
-    r->net_read_desired  = r->net_read_desired  || src->net_read_desired;
-    r->net_write_desired = r->net_write_desired || src->net_write_desired;
-    r->tick_deadline     = ossl_time_min(r->tick_deadline, src->tick_deadline);
+    r->net_read_desired     = r->net_read_desired  || src->net_read_desired;
+    r->net_write_desired    = r->net_write_desired || src->net_write_desired;
+    r->notify_other_threads = r->notify_other_threads || src->notify_other_threads;
+    r->tick_deadline        = ossl_time_min(r->tick_deadline, src->tick_deadline);
 }
 
 struct quic_reactor_st {
@@ -100,6 +102,9 @@ struct quic_reactor_st {
     void (*tick_cb)(QUIC_TICK_RESULT *res, void *arg, uint32_t flags);
     void *tick_cb_arg;
 
+    /* The mutex used for ticking. Not owned by the reactor. */
+    CRYPTO_MUTEX *mutex;
+
     /* Used to notify other threads. Valid only if have_notifier is set. */
     RIO_NOTIFIER notifier;
 
@@ -143,6 +148,7 @@ int ossl_quic_reactor_init(QUIC_REACTOR *rtor,
                            void (*tick_cb)(QUIC_TICK_RESULT *res, void *arg,
                                            uint32_t flags),
                            void *tick_cb_arg,
+                           CRYPTO_MUTEX *mutex,
                            OSSL_TIME initial_tick_deadline,
                            uint64_t flags);
 
@@ -218,34 +224,14 @@ RIO_NOTIFIER *ossl_quic_reactor_get0_notifier(QUIC_REACTOR *rtor);
  * mutex is non-NULL, it must be a lock currently held for write; it will be
  * unlocked during any sleep, and then relocked for write afterwards.
  *
- * Precondition:   mutex is NULL or is held for write (unchecked)
- * Postcondition:  mutex is NULL or is held for write (unless
- *                   CRYPTO_THREAD_write_lock fails)
+ * Precondition:   If a reactor mutex is being used, it must be held (unchecked)
+ * Postcondition:  If a reactor mutex is being used, it is held
  */
 #define SKIP_FIRST_TICK     (1U << 0)
 
 int ossl_quic_reactor_block_until_pred(QUIC_REACTOR *rtor,
                                        int (*pred)(void *arg), void *pred_arg,
-                                       uint32_t flags,
-                                       CRYPTO_MUTEX *mutex);
-
-/*
- * ossl_quic_reactor_notify_other_threads
- * --------------------------------------
- *
- * Notify other threads currently blocking in
- * ossl_quic_reactor_block_until_pred() calls that a predicate they are using
- * might now be met due to state changes.
- *
- * This function must be called after state changes which might cause a
- * predicate in another thread to now be met (i.e., ticking). It is a no-op if
- * inter-thread notification is not being used.
- *
- * mutex is required and must be held.
- */
-void ossl_quic_reactor_notify_other_threads(QUIC_REACTOR *rtor,
-                                            CRYPTO_MUTEX *mutex);
-
+                                       uint32_t flags);
 # endif
 
 #endif

diff --git a/ssl/quic/quic_engine.c b/ssl/quic/quic_engine.c
index c6ecde24a3ffd6a8bc06839531158b5decaad368..6272f4b7614c9bfc1b2a9ea93368a486f0297c39 100644 (file)
--- a/ssl/quic/quic_engine.c
+++ b/ssl/quic/quic_engine.c
@@ -56,6 +56,7 @@ void ossl_quic_engine_free(QUIC_ENGINE *qeng)
 static int qeng_init(QUIC_ENGINE *qeng, uint64_t reactor_flags)
 {
     return ossl_quic_reactor_init(&qeng->rtor, qeng_tick, qeng,
+                                  qeng->mutex,
                                   ossl_time_zero(), reactor_flags);
 }
 
@@ -152,9 +153,10 @@ static void qeng_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)
     QUIC_ENGINE *qeng = arg;
     QUIC_PORT *port;
 
-    res->net_read_desired   = 0;
-    res->net_write_desired  = 0;
-    res->tick_deadline      = ossl_time_infinite();
+    res->net_read_desired     = 0;
+    res->net_write_desired    = 0;
+    res->notify_other_threads = 0;
+    res->tick_deadline        = ossl_time_infinite();
 
     if (qeng->inhibit_tick)
         return;
@@ -166,6 +168,4 @@ static void qeng_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)
         ossl_quic_port_subtick(port, &subr, flags);
         ossl_quic_tick_result_merge_into(res, &subr);
     }
-
-    ossl_quic_reactor_notify_other_threads(&qeng->rtor, qeng->mutex);
 }

diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c
index cb52eb949df39e1748d98cba8f580e1c6cc15e66..faf4cdb10c32aab7f676d4787bcc158a1ba522c4 100644 (file)
--- a/ssl/quic/quic_impl.c
+++ b/ssl/quic/quic_impl.c
@@ -549,8 +549,7 @@ static int block_until_pred(QCTX *ctx,
     ossl_quic_engine_set_inhibit_tick(qeng, 0);
 
     rtor = ossl_quic_engine_get0_reactor(qeng);
-    return ossl_quic_reactor_block_until_pred(rtor, pred, pred_arg, flags,
-                                              ossl_quic_engine_get0_mutex(qeng));
+    return ossl_quic_reactor_block_until_pred(rtor, pred, pred_arg, flags);
 }
 
 /*

diff --git a/ssl/quic/quic_port.c b/ssl/quic/quic_port.c
index 594d37cb01aabba93c55d12fb9b58ed0ffeab8a6..e4bafabd805b70775a94dc365e6446ff76efa92a 100644 (file)
--- a/ssl/quic/quic_port.c
+++ b/ssl/quic/quic_port.c
@@ -480,9 +480,10 @@ void ossl_quic_port_subtick(QUIC_PORT *port, QUIC_TICK_RESULT *res,
 {
     QUIC_CHANNEL *ch;
 
-    res->net_read_desired   = ossl_quic_port_is_running(port);
-    res->net_write_desired  = 0;
-    res->tick_deadline      = ossl_time_infinite();
+    res->net_read_desired       = ossl_quic_port_is_running(port);
+    res->net_write_desired      = 0;
+    res->notify_other_threads   = 0;
+    res->tick_deadline          = ossl_time_infinite();
 
     if (!port->engine->inhibit_tick) {
         /* Handle any incoming data from network. */

diff --git a/ssl/quic/quic_reactor.c b/ssl/quic/quic_reactor.c
index cdc90fefd888afaf355ead4553c7e847ada0f391..142818923ec3a2a71c6fa281f0daeb9f59ab3711 100644 (file)
--- a/ssl/quic/quic_reactor.c
+++ b/ssl/quic/quic_reactor.c
@@ -15,10 +15,13 @@
  * Core I/O Reactor Framework
  * ==========================
  */
+static void rtor_notify_other_threads(QUIC_REACTOR *rtor);
+
 int ossl_quic_reactor_init(QUIC_REACTOR *rtor,
                            void (*tick_cb)(QUIC_TICK_RESULT *res, void *arg,
                                            uint32_t flags),
                            void *tick_cb_arg,
+                           CRYPTO_MUTEX *mutex,
                            OSSL_TIME initial_tick_deadline,
                            uint64_t flags)
 {
@@ -32,6 +35,7 @@ int ossl_quic_reactor_init(QUIC_REACTOR *rtor,
 
     rtor->tick_cb           = tick_cb;
     rtor->tick_cb_arg       = tick_cb_arg;
+    rtor->mutex             = mutex;
 
     rtor->cur_blocking_waiters = 0;
 
@@ -144,6 +148,9 @@ int ossl_quic_reactor_tick(QUIC_REACTOR *rtor, uint32_t flags)
     rtor->net_read_desired  = res.net_read_desired;
     rtor->net_write_desired = res.net_write_desired;
     rtor->tick_deadline     = res.tick_deadline;
+    if (res.notify_other_threads)
+        rtor_notify_other_threads(rtor);
+
     return 1;
 }
 
@@ -387,8 +394,18 @@ static int poll_two_descriptors(const BIO_POLL_DESCRIPTOR *r, int r_want_read,
                         notify_rfd, deadline, mutex);
 }
 
-void ossl_quic_reactor_notify_other_threads(QUIC_REACTOR *rtor,
-                                            CRYPTO_MUTEX *mutex)
+/*
+ * Notify other threads currently blocking in
+ * ossl_quic_reactor_block_until_pred() calls that a predicate they are using
+ * might now be met due to state changes.
+ *
+ * This function must be called after state changes which might cause a
+ * predicate in another thread to now be met (i.e., ticking). It is a no-op if
+ * inter-thread notification is not being used.
+ *
+ * The reactor mutex must be held while calling this function.
+ */
+static void rtor_notify_other_threads(QUIC_REACTOR *rtor)
 {
     if (!rtor->have_notifier)
         return;
@@ -421,7 +438,7 @@ void ossl_quic_reactor_notify_other_threads(QUIC_REACTOR *rtor,
     * unsignalling the notifier.
     */
     while (rtor->signalled_notifier)
-        ossl_crypto_condvar_wait(rtor->notifier_cv, mutex);
+        ossl_crypto_condvar_wait(rtor->notifier_cv, rtor->mutex);
 }
 
 /*
@@ -437,8 +454,7 @@ void ossl_quic_reactor_notify_other_threads(QUIC_REACTOR *rtor,
  */
 int ossl_quic_reactor_block_until_pred(QUIC_REACTOR *rtor,
                                        int (*pred)(void *arg), void *pred_arg,
-                                       uint32_t flags,
-                                       CRYPTO_MUTEX *mutex)
+                                       uint32_t flags)
 {
     int res, net_read_desired, net_write_desired, notifier_fd;
     OSSL_TIME tick_deadline;
@@ -472,7 +488,7 @@ int ossl_quic_reactor_block_until_pred(QUIC_REACTOR *rtor,
                                    net_write_desired,
                                    notifier_fd,
                                    tick_deadline,
-                                   mutex);
+                                   rtor->mutex);
 
         assert(rtor->cur_blocking_waiters > 0);
         --rtor->cur_blocking_waiters;
@@ -530,7 +546,7 @@ int ossl_quic_reactor_block_until_pred(QUIC_REACTOR *rtor,
             } else {
                 /* We are not the last waiter out - so wait for that one. */
                 while (rtor->signalled_notifier)
-                    ossl_crypto_condvar_wait(rtor->notifier_cv, mutex);
+                    ossl_crypto_condvar_wait(rtor->notifier_cv, rtor->mutex);
             }
         }