Fix timer lifetime and C++ callback bookkeeping after reset

Makefile

@@ -409,10 +409,12 @@ unittest: prepare libhv
 	$(CC)  -g -Wall -O0 -std=c99   -I. -Ibase -Iprotocol -o bin/ping              unittest/ping_test.c          protocol/icmp.c base/hsocket.c base/htime.c -DPRINT_DEBUG
 	$(CC)  -g -Wall -O0 -std=c99   -I. -Ibase -Iprotocol -o bin/ftp               unittest/ftp_test.c           protocol/ftp.c  base/hsocket.c base/htime.c
 	$(CC)  -g -Wall -O0 -std=c99   -I. -Ibase -Iprotocol -Iutil -o bin/sendmail   unittest/sendmail_test.c      protocol/smtp.c base/hsocket.c base/htime.c util/base64.c
+	$(CC)  -g -Wall -O0 -std=c99   -I. -Ibase -Issl -Ievent -o bin/timer_reset_test unittest/timer_reset_test.c -Llib -lhv -pthread
 	$(CC)  -g -Wall -O0 -std=c99   -I. -Ibase -Issl -Ievent -o bin/hdns_test      unittest/hdns_test.c      -Llib -lhv -pthread
 	$(CC)  -g -Wall -O0 -std=c99   -I. -Ibase -Issl -Ievent -o bin/hdns_benchmark unittest/hdns_benchmark.c -Llib -lhv -pthread
 ifeq ($(WITH_EVPP), yes)
 	$(CXX) -g -Wall -O0 -std=c++11 -I. -Ibase -Issl -Ievent -Icpputil -Ievpp -o bin/tcpclient_dns_test unittest/tcpclient_dns_test.cpp -Llib -lhv -pthread
+	$(CXX) -g -Wall -O0 -std=c++11 -I. -Ibase -Issl -Ievent -Icpputil -Ievpp -o bin/eventloop_timer_test unittest/eventloop_timer_test.cpp -Llib -lhv -pthread
 endif
 ifeq ($(WITH_HTTP), yes)
 	$(CXX) -g -Wall -O0 -std=c++11 -I. -Ibase -Ihttp -o bin/websocket_parser_test unittest/websocket_parser_test.cpp -Llib -lhv -pthread

event/hevent.h

@@ -106,11 +106,12 @@ struct hidle_s {
     struct list_node node;
 };
 
-#define HTIMER_FIELDS                   \
-    HEVENT_FIELDS                       \
-    uint32_t    repeat;                 \
-    uint64_t    next_timeout;           \
-    struct heap_node node;
+#define HTIMER_FIELDS      \
+    HEVENT_FIELDS          \
+    uint32_t repeat;       \
+    uint64_t next_timeout; \
+    struct heap_node node; \
+    unsigned pending_reset : 1;
 
 struct htimer_s {
     HTIMER_FIELDS
@@ -313,13 +314,12 @@ void hio_memmove_readbuf(hio_t* io);
         }\
     } while(0)
 
-#define EVENT_RESET(ev) \
-    do {\
-        ev->destroy = 0;\
-        EVENT_ACTIVE(ev);\
-        ev->pending = 0;\
-    } while(0)
-
+#define EVENT_RESET(ev)   \
+    do {                  \
+        ev->destroy = 0;  \
+        EVENT_ACTIVE(ev); \
+    } while (0)
+
 #define EVENT_UNPENDING(ev) \
     do {\
         if (ev->pending) {\

event/hloop.c

@@ -118,10 +118,15 @@ static int hloop_process_pendings(hloop_t* loop) {
         while (cur) {
             next = cur->pending_next;
             if (cur->pending && cur->loop == loop) {
-                if (cur->active && cur->cb) {
+                // A reset cancels delivery, not the pending allocation reference.
+                bool reset_timer = (cur->event_type & HEVENT_TYPE_TIMER) && ((htimer_t*)cur)->pending_reset;
+                if (cur->active && cur->cb && !reset_timer) {
                     cur->cb(cur);
                     ++ncbs;
                 }
+                if (cur->event_type & HEVENT_TYPE_TIMER) {
+                    ((htimer_t*)cur)->pending_reset = 0;
+                }
                 cur->pending = 0;
                 // NOTE: Now we can safely delete event marked as destroy.
                 if (cur->destroy) {
@@ -756,6 +761,8 @@ void htimer_reset(htimer_t* timer, uint32_t timeout_ms) {
         timer->next_timeout = timer->next_timeout / 100000 * 100000;
     }
     heap_insert(&loop->timers, &timer->node);
+    // Keep pending ownership until dispatch releases its current/next pointer.
+    timer->pending_reset = timer->pending;
     EVENT_RESET(timer);
 }
 
@@ -799,6 +806,10 @@ void htimer_del(htimer_t* timer) {
     EVENT_DEL(timer);
 }
 
+uint32_t htimer_repeat(const htimer_t* timer) {
+    return timer->repeat;
+}
+
 const char* hio_engine() {
 #ifdef EVENT_SELECT
     return  "select";

event/hloop.h

@@ -236,6 +236,21 @@ HV_EXPORT htimer_t* htimer_add_period(hloop_t* loop, htimer_cb cb,
 
 HV_EXPORT void htimer_del(htimer_t* timer);
 HV_EXPORT void htimer_reset(htimer_t* timer, uint32_t timeout_ms DEFAULT(0));
+/*
+ * Returns the native remaining expiration count of a timer, or INFINITE.
+ *
+ * Contract:
+ * - The caller must supply a valid, live timer handle; the function does not
+ *   validate pointers or make using a freed timer safe.
+ * - Must be read under the timer's normal owner-loop/thread rules. The getter
+ *   provides no cross-thread synchronization.
+ * - A finite repetition is consumed when its expiration becomes pending,
+ *   before callback delivery. Resetting away that pending callback does not
+ *   refund it.
+ * - A final callback can observe zero while dispatch still keeps the timer
+ *   alive. Resetting that live timer can revive its count from zero to one.
+ */
+HV_EXPORT uint32_t htimer_repeat(const htimer_t* timer);
 
 // io
 //-----------------------low-level apis---------------------------------------

evpp/EventLoop.h

@@ -159,13 +159,11 @@ class EventLoop : public Status, public std::enable_shared_from_this<EventLoop>
 
     // resetTimer thread-safe
     void resetTimer(TimerID timerID, int timeout_ms = 0) {
-        runInLoop([timerID, timeout_ms, this](){
+        runInLoop([timerID, timeout_ms, this]() {
             auto iter = timers.find(timerID);
             if (iter != timers.end()) {
                 htimer_reset(iter->second->timer, timeout_ms);
-                if (iter->second->repeat == 0) {
-                    iter->second->repeat = 1;
-                }
+                iter->second->repeat = htimer_repeat(iter->second->timer);
             }
         });
     }
@@ -267,7 +265,10 @@ class EventLoop : public Status, public std::enable_shared_from_this<EventLoop>
             if (timer->cb) timer->cb(timerID);
             if (timer->repeat == 0) {
                 // htimer_t alloc and free by hloop, but timers[timerID] managed by EventLoop.
-                loop->timers.erase(timerID);
+                auto current = loop->timers.find(timerID);
+                if (current != loop->timers.end() && current->second == timer) {
+                    loop->timers.erase(current);
+                }
             }
         }
     }

scripts/unittest.sh

@@ -32,6 +32,9 @@ bin/socketpair_test
 # bin/objectpool_test
 bin/sizeof_test
 bin/http_router_test
+if [ -x bin/timer_reset_test ]; then
+    bin/timer_reset_test || exit $?
+fi
 if [ -x bin/websocket_parser_test ]; then
     bin/websocket_parser_test || exit $?
 fi
@@ -77,6 +80,9 @@ fi
 if [ -x bin/tcpclient_dns_test ]; then
     bin/tcpclient_dns_test
 fi
+if [ -x bin/eventloop_timer_test ]; then
+    bin/eventloop_timer_test || exit $?
+fi
 for redis_test in redis_async_client_test redis_client_test redis_batch_test redis_subscriber_test lua_redis_test; do
     if [ -x bin/${redis_test} ]; then
         if [ "${redis_test}" = "lua_redis_test" ]; then

unittest/CMakeLists.txt

@@ -168,9 +168,15 @@ if(WIN32 AND WITH_WEPOLL)
 add_executable(epoll_init_test epoll_init_test.c ../event/wepoll/wepoll.c)
 target_include_directories(epoll_init_test PRIVATE .. ../base ../ssl ../event)
 target_link_libraries(epoll_init_test ${HV_LIBRARIES})
-set(EVENT_UNITTEST_TARGETS epoll_init_test)
+list(APPEND EVENT_UNITTEST_TARGETS epoll_init_test)
 endif()
 
+# ------event: timer reset and lifetime------
+add_executable(timer_reset_test timer_reset_test.c)
+target_include_directories(timer_reset_test PRIVATE .. ../base ../ssl ../event)
+target_link_libraries(timer_reset_test ${HV_LIBRARIES})
+list(APPEND EVENT_UNITTEST_TARGETS timer_reset_test)
+
 # ------event: async dns------
 add_executable(hdns_test hdns_test.c)
 target_include_directories(hdns_test PRIVATE .. ../base ../ssl ../event)
@@ -198,12 +204,17 @@ endforeach()
 list(APPEND REDIS_UNITTEST_TARGETS ${REDIS_LINKED_UNITTEST_TARGETS})
 endif()
 
-# ------evpp: async dns in connect path (connect/reconnect, lifetime, resolve-fail)------
+# ------evpp: async dns and timer lifetime------
 if(WITH_EVPP)
 add_executable(tcpclient_dns_test tcpclient_dns_test.cpp)
 target_include_directories(tcpclient_dns_test PRIVATE .. ../base ../ssl ../event ../cpputil ../evpp)
 target_link_libraries(tcpclient_dns_test ${HV_LIBRARIES})
-set(EVPP_DNS_UNITTEST_TARGETS tcpclient_dns_test)
+list(APPEND EVPP_UNITTEST_TARGETS tcpclient_dns_test)
+
+add_executable(eventloop_timer_test eventloop_timer_test.cpp)
+target_include_directories(eventloop_timer_test PRIVATE .. ../base ../ssl ../event ../cpputil ../evpp)
+target_link_libraries(eventloop_timer_test ${HV_LIBRARIES})
+list(APPEND EVPP_UNITTEST_TARGETS eventloop_timer_test)
 endif()
 
 if(UNIX)
@@ -243,5 +254,5 @@ add_custom_target(unittest DEPENDS
     hdns_test
     hdns_benchmark
     ${REDIS_UNITTEST_TARGETS}
-    ${EVPP_DNS_UNITTEST_TARGETS}
+    ${EVPP_UNITTEST_TARGETS}
 )