Enhance System::waitSignal(), allow to listen multiple signals simult…

…aneously
swoole · Nov 18, 2024 · c4b3271 · c4b3271
1 parent 85d53fe
commit c4b3271
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 69 deletions.
diff --git a/ext-src/stubs/php_swoole_coroutine_system.stub.php b/ext-src/stubs/php_swoole_coroutine_system.stub.php
@@ -11,7 +11,7 @@ public static function readFile(string $filename, int $flag = 0): false|string {
         public static function writeFile(string $filename, string $fileContent, int $flags = 0): false|int {}
         public static function wait(float $timeout = -1): array|false {}
         public static function waitPid(int $pid, float $timeout = -1): array|false {}
-        public static function waitSignal(int $signo, float $timeout = -1): bool {}
+        public static function waitSignal(int|array $signals, float $timeout = -1): bool {}
         public static function waitEvent(mixed $socket, int $events = SWOOLE_EVENT_READ, float $timeout = -1): int|false {}
     }
 }
diff --git a/ext-src/stubs/php_swoole_coroutine_system_arginfo.h b/ext-src/stubs/php_swoole_coroutine_system_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 8c900b375f6550820f059936b75e7a80beb088f5 */
+ * Stub hash: 59e0a163279f6b7bf53de6f8f50a8ba4820ccf3b */
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_Swoole_Coroutine_System_gethostbyname, 0, 1, MAY_BE_FALSE|MAY_BE_STRING)
 	ZEND_ARG_TYPE_INFO(0, domain_name, IS_STRING, 0)
@@ -56,7 +56,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_Swoole_Coroutine_System_wa
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Swoole_Coroutine_System_waitSignal, 0, 1, _IS_BOOL, 0)
-	ZEND_ARG_TYPE_INFO(0, signo, IS_LONG, 0)
+	ZEND_ARG_TYPE_MASK(0, signals, MAY_BE_LONG|MAY_BE_ARRAY, NULL)
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, timeout, IS_DOUBLE, 0, "-1")
 ZEND_END_ARG_INFO()
 

diff --git a/ext-src/swoole_coroutine_system.cc b/ext-src/swoole_coroutine_system.cc
@@ -335,20 +335,32 @@ PHP_METHOD(swoole_coroutine_system, waitPid) {
 
 PHP_METHOD(swoole_coroutine_system, waitSignal) {
     SW_MUST_BE_MAIN_THREAD();
-    zend_long signo;
+    zval *zsignals;
     double timeout = -1;
 
     ZEND_PARSE_PARAMETERS_START(1, 2)
-    Z_PARAM_LONG(signo)
+    Z_PARAM_ZVAL(zsignals)
     Z_PARAM_OPTIONAL
     Z_PARAM_DOUBLE(timeout)
     ZEND_PARSE_PARAMETERS_END_EX(RETURN_FALSE);
 
-    if (!System::wait_signal(signo, timeout)) {
+    std::vector<int> signals;
+
+    if (ZVAL_IS_ARRAY(zsignals)) {
+        zval *item;
+        ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(zsignals), item) {
+            signals.push_back(zval_get_long(item));
+        }
+        ZEND_HASH_FOREACH_END();
+    } else {
+        signals.push_back(zval_get_long(zsignals));
+    }
+
+    if (!System::wait_signal(signals, timeout)) {
         if (swoole_get_last_error() == EBUSY) {
             php_swoole_fatal_error(E_WARNING, "Unable to wait signal, async signal listener has been registered");
         } else if (swoole_get_last_error() == EINVAL) {
-            php_swoole_fatal_error(E_WARNING, "Invalid signal [" ZEND_LONG_FMT "]", signo);
+            php_swoole_fatal_error(E_WARNING, "Invalid signal in the given list");
         }
         errno = swoole_get_last_error();
         RETURN_FALSE;

diff --git a/include/swoole_coroutine_system.h b/include/swoole_coroutine_system.h
@@ -67,16 +67,18 @@ class System {
     static pid_t wait(int *__stat_loc, double timeout = -1);
     static pid_t waitpid(pid_t __pid, int *__stat_loc, int __options, double timeout = -1);
     /* signal */
-    static bool wait_signal(int signo, double timeout = -1);
+    static bool wait_signal(int signal, double timeout = -1);
+    static bool wait_signal(const std::vector<int> &signals, double timeout = -1);
     /* event */
     static int wait_event(int fd, int events, double timeout);
 };
 std::string gethostbyname_impl_with_async(const std::string &hostname, int domain, double timeout = -1);
 //-------------------------------------------------------------------------------
 struct AsyncLock {
- private:
+  private:
     void *resource_;
- public:
+
+  public:
     AsyncLock(void *resource);
     ~AsyncLock();
 };

diff --git a/src/coroutine/system.cc b/src/coroutine/system.cc
@@ -223,94 +223,70 @@ std::vector<std::string> System::getaddrinfo(
     return retval;
 }
 
+bool System::wait_signal(int signal, double timeout) {
+    std::vector<int> signals = {signal};
+    return wait_signal(signals, timeout);
+}
+
 /**
  * @error: swoole_get_last_error()
  */
-bool System::wait_signal(int signo, double timeout) {
+bool System::wait_signal(const std::vector<int> &signals, double timeout) {
     static Coroutine *listeners[SW_SIGNO_MAX];
     Coroutine *co = Coroutine::get_current_safe();
 
     if (SwooleTG.signal_listener_num > 0) {
         swoole_set_last_error(EBUSY);
         return false;
     }
-    if (signo < 0 || signo >= SW_SIGNO_MAX || signo == SIGCHLD) {
-        swoole_set_last_error(EINVAL);
-        return false;
+
+    auto callback_fn = [](int signo) {
+        Coroutine *co = listeners[signo];
+        if (co) {
+            listeners[signo] = nullptr;
+            co->resume();
+        }
+    };
+
+    for (auto &signo : signals) {
+        if (signo < 0 || signo >= SW_SIGNO_MAX || signo == SIGCHLD) {
+            swoole_set_last_error(EINVAL);
+            return false;
+        }
+
+        /* resgiter signal */
+        listeners[signo] = co;
+
+#ifdef SW_USE_THREAD_CONTEXT
+        swoole_event_defer([signo](void *) { swoole_signal_set(signo, callback_fn, nullptr) });
+#else
+        swoole_signal_set(signo, callback_fn);
+#endif
     }
 
-    /* resgiter signal */
-    listeners[signo] = co;
     // exit condition
     if (!sw_reactor()->isset_exit_condition(Reactor::EXIT_CONDITION_CO_SIGNAL_LISTENER)) {
         sw_reactor()->set_exit_condition(
             Reactor::EXIT_CONDITION_CO_SIGNAL_LISTENER,
             [](Reactor *reactor, size_t &event_num) -> bool { return SwooleTG.co_signal_listener_num == 0; });
     }
 
-#ifdef SW_USE_THREAD_CONTEXT
-    swoole_event_defer(
-        [signo](void *) {
-            swoole_signal_set(signo, [](int signo) {
-                Coroutine *co = listeners[signo];
-                if (co) {
-                    listeners[signo] = nullptr;
-                    co->resume();
-                }
-            });
-        },
-        nullptr);
-#else
-    swoole_signal_set(signo, [](int signo) {
-        Coroutine *co = listeners[signo];
-        if (co) {
-            listeners[signo] = nullptr;
-            co->resume();
-        }
-    });
-#endif
-
     SwooleTG.co_signal_listener_num++;
 
-    TimerNode *timer = nullptr;
-    if (timeout > 0) {
-        timer = swoole_timer_add(
-            timeout,
-            0,
-            [](Timer *timer, TimerNode *tnode) {
-                Coroutine *co = (Coroutine *) tnode->data;
-                co->resume();
-            },
-            co);
-    }
-
-    Coroutine::CancelFunc cancel_fn = [timer](Coroutine *co) {
-        if (timer) {
-            swoole_timer_del(timer);
-        }
-        co->resume();
-        return true;
-    };
-    co->yield(&cancel_fn);
+    bool retval = co->yield_ex(timeout);
 
+    for (auto &signo : signals) {
 #ifdef SW_USE_THREAD_CONTEXT
-    swoole_event_defer([signo](void *) { swoole_signal_set(signo, nullptr); }, nullptr);
+        swoole_event_defer([signo](void *) { swoole_signal_set(signo, nullptr); }, nullptr);
 #else
-    swoole_signal_set(signo, nullptr);
+        swoole_signal_set(signo, nullptr);
 #endif
-    SwooleTG.co_signal_listener_num--;
-
-    if (listeners[signo] != nullptr) {
         listeners[signo] = nullptr;
-        swoole_set_last_error(co->is_canceled() ? SW_ERROR_CO_CANCELED : ETIMEDOUT);
-        return false;
     }
 
-    if (timer) {
-        swoole_timer_del(timer);
-    }
+    SwooleTG.co_signal_listener_num--;
 
-    return !co->is_canceled();
+    return retval;
 }
 
 struct CoroPollTask {

diff --git a/tests/swoole_coroutine_system/waitSignal_2.phpt b/tests/swoole_coroutine_system/waitSignal_2.phpt
@@ -0,0 +1,58 @@
+--TEST--
+swoole_coroutine_system: waitSignal 2
+--SKIPIF--
+<?php require __DIR__ . '/../include/skipif.inc'; ?>
+--FILE--
+<?php
+require __DIR__ . '/../include/bootstrap.php';
+
+use Swoole\Atomic;
+use Swoole\Coroutine;
+use Swoole\Coroutine\System;
+use Swoole\Process;
+
+$atomic = new Atomic;
+
+$pid = getmypid();
+$killer = new Process(function () use ($pid, $atomic) {
+    $atomic->wait();
+    echo "2\n";
+    switch_process();
+    Process::kill($pid, SIGUSR1);
+    $atomic->wait();
+    echo "6\n";
+    switch_process();
+    Process::kill($pid, SIGUSR2);
+    echo "8\n";
+});
+$killer->start();
+
+Coroutine\run(function () use ($atomic) {
+    Coroutine::sleep(0.001);
+    switch_process();
+    $atomic->wakeup();
+    echo "1\n";
+    $list = [SIGUSR1, SIGUSR2, SIGIO];
+    Assert::true(System::waitSignal($list));
+    echo "3\n";
+    Assert::false(System::waitSignal($list, 0.01));
+    echo "4\n";
+    $atomic->wakeup();
+    echo "5\n";
+    Assert::true(System::waitSignal($list));
+    echo "7\n";
+    System::wait();
+    echo "9\n";
+});
+
+?>
+--EXPECT--
+1
+2
+3
+4
+5
+6
+8
+7
+9