Fix bug #4794 (#4807)

* fix #4794 * fix #4794 * fix #4794 * delete some tests * fix bug #4794 * fix bug #4794 * fix bug #4794 * fix bug #4794
swoole · Aug 19, 2022 · 4c95a3d · 4c95a3d
1 parent 338c320
commit 4c95a3d
Show file tree

Hide file tree

Showing 24 changed files with 309 additions and 38 deletions.
diff --git a/core-tests/src/os/timer.cpp b/core-tests/src/os/timer.cpp
@@ -32,10 +32,10 @@ TEST(timer, sys) {
     uint64_t ms1 = swoole::time<std::chrono::milliseconds>();
 
     swoole_timer_add(
-        20, false, [&](Timer *, TimerNode *) { timer1_count++; }, nullptr);
+        20L, false, [&](Timer *, TimerNode *) { timer1_count++; }, nullptr);
 
     swoole_timer_add(
-        100,
+        100L,
         true,
         [&](Timer *, TimerNode *tnode) {
             timer2_count++;

diff --git a/ext-src/swoole_server_port.cc b/ext-src/swoole_server_port.cc
@@ -388,7 +388,8 @@ static PHP_METHOD(swoole_server_port, set) {
         port->open_redis_protocol = zval_get_long(ztmp);
     }
     if (php_swoole_array_get_value(vht, "max_idle_time", ztmp)) {
-        port->max_idle_time = zval_get_double(ztmp);
+        double v = zval_get_double(ztmp);
+        port->max_idle_time = SW_MAX(v, SW_TIMER_MIN_SEC);
     }
     // tcp_keepidle
     if (php_swoole_array_get_value(vht, "tcp_keepidle", ztmp)) {

diff --git a/ext-src/swoole_timer.cc b/ext-src/swoole_timer.cc
@@ -154,7 +154,7 @@ static void timer_add(INTERNAL_FUNCTION_PARAMETERS, bool persistent) {
         php_swoole_check_reactor();
     }
 
-    tnode = swoole_timer_add(ms, persistent, timer_callback, fci);
+    tnode = swoole_timer_add((long) ms, persistent, timer_callback, fci);
     if (UNEXPECTED(!tnode)) {
         php_swoole_fatal_error(E_WARNING, "add timer failed");
         goto _failed;

diff --git a/include/swoole_api.h b/include/swoole_api.h
@@ -27,7 +27,13 @@ enum swEventInitFlag {
 
 SW_API long swoole_timer_after(long ms, const swoole::TimerCallback &callback, void *private_data = nullptr);
 SW_API long swoole_timer_tick(long ms, const swoole::TimerCallback &callback, void *private_data = nullptr);
-SW_API swoole::TimerNode *swoole_timer_add(long ms, bool persistent, const swoole::TimerCallback &callback,
+SW_API swoole::TimerNode *swoole_timer_add(double ms,
+                                           bool persistent,
+                                           const swoole::TimerCallback &callback,
+                                           void *private_data = nullptr);
+SW_API swoole::TimerNode *swoole_timer_add(long ms,
+                                           bool persistent,
+                                           const swoole::TimerCallback &callback,
                                            void *private_data = nullptr);
 SW_API bool swoole_timer_del(swoole::TimerNode *tnode);
 SW_API bool swoole_timer_exists(long timer_id);

diff --git a/include/swoole_coroutine_socket.h b/include/swoole_coroutine_socket.h
@@ -505,7 +505,7 @@ class Socket {
             if (timeout != 0 && !*timer_pp) {
                 enabled = true;
                 if (timeout > 0) {
-                    *timer_pp = swoole_timer_add((long) (timeout * 1000), false, callback, socket_);
+                    *timer_pp = swoole_timer_add(timeout, false, callback, socket_);
                     return *timer_pp != nullptr;
                 }
                 *timer_pp = (TimerNode *) -1;

diff --git a/include/swoole_server.h b/include/swoole_server.h
@@ -346,7 +346,7 @@ struct ListenPort {
 
     int tcp_user_timeout = 0;
 
-    uint16_t max_idle_time = 0;
+    double max_idle_time = 0;
 
     int socket_buffer_size = network::Socket::default_buffer_size;
     uint32_t buffer_high_watermark = 0;

diff --git a/src/coroutine/base.cc b/src/coroutine/base.cc
@@ -79,7 +79,7 @@ bool Coroutine::yield_ex(double timeout) {
     };
 
     if (timeout > 0) {
-        timer = swoole_timer_add((long) (timeout * 1000), false, timer_callback, nullptr);
+        timer = swoole_timer_add(timeout, false, timer_callback, nullptr);
     }
 
     CancelFunc cancel_fn = [](Coroutine *co) {

diff --git a/src/coroutine/channel.cc b/src/coroutine/channel.cc
@@ -63,11 +63,10 @@ void *Channel::pop(double timeout) {
         msg.error = false;
         msg.timer = nullptr;
         if (timeout > 0) {
-            long msec = (long) (timeout * 1000);
             msg.chan = this;
             msg.type = CONSUMER;
             msg.co = current_co;
-            msg.timer = swoole_timer_add(msec, false, timer_callback, &msg);
+            msg.timer = swoole_timer_add(timeout, false, timer_callback, &msg);
         }
 
         yield(CONSUMER);
@@ -114,11 +113,10 @@ bool Channel::push(void *data, double timeout) {
         msg.error = false;
         msg.timer = nullptr;
         if (timeout > 0) {
-            long msec = (long) (timeout * 1000);
             msg.chan = this;
             msg.type = PRODUCER;
             msg.co = current_co;
-            msg.timer = swoole_timer_add(msec, false, timer_callback, &msg);
+            msg.timer = swoole_timer_add(timeout, false, timer_callback, &msg);
         }
 
         yield(PRODUCER);

diff --git a/src/coroutine/system.cc b/src/coroutine/system.cc
@@ -277,7 +277,7 @@ bool System::wait_signal(int signo, double timeout) {
     TimerNode *timer = nullptr;
     if (timeout > 0) {
         timer = swoole_timer_add(
-            timeout * 1000,
+            timeout,
             0,
             [](Timer *timer, TimerNode *tnode) {
                 Coroutine *co = (Coroutine *) tnode->data;
@@ -465,10 +465,7 @@ bool System::socket_poll(std::unordered_map<int, PollSocket> &fds, double timeou
     }
 
     if (timeout > 0) {
-        if (timeout < 0.001) {
-            timeout = 0.001;
-        }
-        task.timer = swoole_timer_add((long) (timeout * 1000), false, socket_poll_timeout, &task);
+        task.timer = swoole_timer_add(timeout, false, socket_poll_timeout, &task);
     }
 
     task.co->yield();
@@ -505,7 +502,7 @@ struct EventWaiter {
         }
 
         if (timeout > 0) {
-            timer = swoole_timer_add((long) (timeout * 1000),
+            timer = swoole_timer_add(timeout,
                                      false,
                                      [](Timer *timer, TimerNode *tnode) {
                                          EventWaiter *waiter = (EventWaiter *) tnode->data;

diff --git a/src/coroutine/thread_context.cc b/src/coroutine/thread_context.cc
@@ -32,9 +32,13 @@ static std::mutex *current_lock = nullptr;
 
 void thread_context_init() {
     if (!swoole_timer_is_available()) {
-        swoole_timer_add(1, false, [](Timer *timer, TimerNode *tnode) {
-            // do nothing
-        }, nullptr);
+        swoole_timer_add(
+            1L,
+            false,
+            [](Timer *timer, TimerNode *tnode) {
+                // do nothing
+            },
+            nullptr);
     }
     if (SwooleTG.async_threads == nullptr) {
         SwooleTG.async_threads = new AsyncThreads();

diff --git a/src/network/client.cc b/src/network/client.cc
@@ -661,7 +661,7 @@ static int Client_tcp_connect_async(Client *cli, const char *host, int port, dou
             return SW_ERR;
         }
         if (timeout > 0) {
-            cli->timer = swoole_timer_add((long) (timeout * 1000), false, Client_onTimeout, cli);
+            cli->timer = swoole_timer_add(timeout, false, Client_onTimeout, cli);
         }
         return SW_OK;
     } else {

diff --git a/src/os/wait.cc b/src/os/wait.cc
@@ -137,7 +137,7 @@ pid_t System::waitpid(pid_t __pid, int *__stat_loc, int __options, double timeou
     TimerNode *timer = nullptr;
     if (timeout > 0) {
         timer = swoole_timer_add(
-            timeout * 1000,
+            timeout,
             false,
             [](Timer *timer, TimerNode *tnode) {
                 Coroutine *co = (Coroutine *) tnode->data;

diff --git a/src/server/master.cc b/src/server/master.cc
@@ -55,7 +55,7 @@ TimerCallback Server::get_timeout_callback(ListenPort *port, Reactor *reactor, C
 
 void Server::disable_accept() {
     enable_accept_timer = swoole_timer_add(
-        SW_ACCEPT_RETRY_TIME * 1000,
+        SW_ACCEPT_RETRY_TIME,
         false,
         [](Timer *timer, TimerNode *tnode) {
             Server *serv = (Server *) tnode->data;
@@ -237,7 +237,7 @@ int Server::connection_incoming(Reactor *reactor, Connection *conn) {
     if (port->max_idle_time > 0) {
         auto timeout_callback = get_timeout_callback(port, reactor, conn);
         conn->socket->recv_timeout_ = port->max_idle_time;
-        conn->socket->recv_timer = swoole_timer_add(port->max_idle_time * 1000, true, timeout_callback);
+        conn->socket->recv_timer = swoole_timer_add((long) (port->max_idle_time * 1000), true, timeout_callback);
     }
 #ifdef SW_USE_OPENSSL
     if (conn->socket->ssl) {
@@ -443,7 +443,7 @@ int Server::start_master_thread() {
         reactor->add(pipe_command->get_socket(true), SW_EVENT_READ);
     }
 
-    if ((master_timer = swoole_timer_add(1000, true, Server::timer_callback, this)) == nullptr) {
+    if ((master_timer = swoole_timer_add(1000L, true, Server::timer_callback, this)) == nullptr) {
         swoole_event_free();
         return SW_ERR;
     }
@@ -1402,19 +1402,16 @@ int Server::send_to_connection(SendData *_send) {
     else {
         // connection is closed
         if (conn->peer_closed) {
-            swoole_error_log(
-                SW_LOG_NOTICE, SW_ERROR_SESSION_CLOSED_BY_CLIENT, "socket#%d is closed by client", fd);
+            swoole_error_log(SW_LOG_NOTICE, SW_ERROR_SESSION_CLOSED_BY_CLIENT, "socket#%d is closed by client", fd);
             return false;
         }
         // connection output buffer overflow
         if (_socket->out_buffer->length() >= _socket->buffer_size) {
             if (send_yield) {
                 swoole_set_last_error(SW_ERROR_OUTPUT_SEND_YIELD);
             } else {
-                swoole_error_log(SW_LOG_WARNING,
-                                 SW_ERROR_OUTPUT_BUFFER_OVERFLOW,
-                                 "connection#%d output buffer overflow",
-                                 fd);
+                swoole_error_log(
+                    SW_LOG_WARNING, SW_ERROR_OUTPUT_BUFFER_OVERFLOW, "connection#%d output buffer overflow", fd);
             }
             conn->overflow = 1;
             if (onBufferEmpty && onBufferFull == nullptr) {
@@ -1436,7 +1433,7 @@ int Server::send_to_connection(SendData *_send) {
         auto timeout_callback = get_timeout_callback(port, reactor, conn);
         _socket->send_timeout_ = port->max_idle_time;
         _socket->last_sent_time = time<std::chrono::milliseconds>(true);
-        _socket->send_timer = swoole_timer_add(port->max_idle_time * 1000, true, timeout_callback);
+        _socket->send_timer = swoole_timer_add((long) (port->max_idle_time * 1000), true, timeout_callback);
     }
 
     if (!_socket->isset_writable_event()) {
@@ -1483,7 +1480,7 @@ bool Server::sendfile(SessionId session_id, const char *file, uint32_t l_file, o
                          "sendfile name[%.8s...] length %u is exceed the max name len %u",
                          file,
                          l_file,
-                         (uint32_t)(SW_IPC_BUFFER_SIZE - sizeof(SendfileTask) - 1));
+                         (uint32_t) (SW_IPC_BUFFER_SIZE - sizeof(SendfileTask) - 1));
         return false;
     }
     // string must be zero termination (for `state` system call)
@@ -1737,7 +1734,7 @@ ListenPort *Server::add_port(SocketType type, const char *host, int port) {
 
 #ifdef SW_USE_OPENSSL
     if (type & SW_SOCK_SSL) {
-        type = (SocketType)(type & (~SW_SOCK_SSL));
+        type = (SocketType) (type & (~SW_SOCK_SSL));
         ls->type = type;
         ls->ssl = 1;
         ls->ssl_context = new SSLContext();

diff --git a/src/server/reactor_process.cc b/src/server/reactor_process.cc
@@ -327,7 +327,7 @@ static int ReactorProcess_loop(ProcessPool *pool, Worker *worker) {
         }
     }
 
-    if ((serv->master_timer = swoole_timer_add(1000, true, Server::timer_callback, serv)) == nullptr) {
+    if ((serv->master_timer = swoole_timer_add(1000L, true, Server::timer_callback, serv)) == nullptr) {
     _fail:
         swoole_event_free();
         return SW_ERR;

diff --git a/src/server/reactor_thread.cc b/src/server/reactor_thread.cc
@@ -568,7 +568,8 @@ static int ReactorThread_onRead(Reactor *reactor, Event *event) {
     }
     if (serv->is_process_mode() && serv->max_queued_bytes && conn->recv_queued_bytes > serv->max_queued_bytes) {
         conn->waiting_time = 1;
-        conn->timer = swoole_timer_add(conn->waiting_time, false, ReactorThread_resume_data_receiving, event->socket);
+        conn->timer =
+            swoole_timer_add((long) conn->waiting_time, false, ReactorThread_resume_data_receiving, event->socket);
         if (conn->timer) {
             reactor->remove_read_event(event->socket);
         }
@@ -888,7 +889,7 @@ static void ReactorThread_resume_data_receiving(Timer *timer, TimerNode *tnode)
         if (conn->waiting_time != 1024) {
             conn->waiting_time *= 2;
         }
-        conn->timer = swoole_timer_add(conn->waiting_time, false, ReactorThread_resume_data_receiving, _socket);
+        conn->timer = swoole_timer_add((long) conn->waiting_time, false, ReactorThread_resume_data_receiving, _socket);
         if (conn->timer) {
             return;
         }

diff --git a/src/wrapper/timer.cc b/src/wrapper/timer.cc
@@ -29,6 +29,14 @@ bool swoole_timer_is_available() {
     return SwooleTG.timer != nullptr;
 }
 
+TimerNode *swoole_timer_add(double timeout, bool persistent, const TimerCallback &callback, void *private_data) {
+    if (timeout < SW_TIMER_MIN_SEC) {
+        return swoole_timer_add(1L, persistent, callback, private_data);
+    }
+
+    return swoole_timer_add((long) (timeout * 1000), persistent, callback, private_data);
+}
+
 TimerNode *swoole_timer_add(long ms, bool persistent, const TimerCallback &callback, void *private_data) {
     if (sw_unlikely(!swoole_timer_is_available())) {
         SwooleTG.timer = new Timer();

diff --git a/tests/swoole_timer/bug_4794.phpt b/tests/swoole_timer/bug_4794.phpt
@@ -0,0 +1,28 @@
+--TEST--
+swoole_timer: #4794 Timer::add() (ERRNO 505): msec value[0] is invalid
+--SKIPIF--
+<?php require __DIR__ . '/../include/skipif.inc'; ?>
+--FILE--
+<?php
+require __DIR__ . '/../include/bootstrap.php';
+use Swoole\Coroutine;
+use Swoole\Coroutine\Channel;
+use function Swoole\Coroutine\run;
+use function Swoole\Coroutine\go;
+
+run(function(){
+    $channel = new Channel(1);
+    go(function () use ($channel) {
+        $channel->push(['rand' => 9999]);
+    });
+    go(function () use ($channel) {
+        $data = $channel->pop(0.00001);
+        var_dump($data);
+    });
+});
+?>
+--EXPECT--
+array(1) {
+  ["rand"]=>
+  int(9999)
+}
diff --git a/tests/swoole_timer/bug_4794_2.phpt b/tests/swoole_timer/bug_4794_2.phpt
@@ -0,0 +1,59 @@
+--TEST--
+swoole_timer: #4794 Timer::add() (ERRNO 505): msec value[0] is invalid
+--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;
+
+$processFast = new Process(function () {
+    usleep(1000);
+});
+$processFast->start();
+
+$processSlow = new Process(function () use ($atomic) {
+    $atomic->wait(10);
+    usleep(10 * 1000);
+});
+$processSlow->start();
+
+Coroutine\run(function () use ($processFast, $processSlow, $atomic) {
+    for ($n = MAX_REQUESTS; $n--;) {
+        $status = System::waitPid($processSlow->pid, 0.0001);
+        Assert::false($status);
+        Assert::same(swoole_last_error(), SOCKET_ETIMEDOUT);
+    }
+    $atomic->wakeup();
+    $status = System::waitPid($processSlow->pid, 1);
+    Assert::same($status['pid'], $processSlow->pid);
+    var_dump($status);
+    $status = System::waitPid($processFast->pid);
+    Assert::same($status['pid'], $processFast->pid);
+    var_dump($status);
+});
+
+?>
+--EXPECTF--
+array(3) {
+  ["pid"]=>
+  int(%d)
+  ["code"]=>
+  int(0)
+  ["signal"]=>
+  int(0)
+}
+array(3) {
+  ["pid"]=>
+  int(%d)
+  ["code"]=>
+  int(0)
+  ["signal"]=>
+  int(0)
+}