From 04d5a9e4b20e37a6b79557315876207a3b7d5758 Mon Sep 17 00:00:00 2001 From: ithewei Date: Fri, 22 May 2026 12:09:24 +0800 Subject: [PATCH 1/9] fix: apply suggestions for base dir from ai code review --- base/hbase.c | 10 +++++++++- base/hbase.h | 2 +- base/hdef.h | 2 +- base/hlog.c | 42 +++++++++++++++++++++++++----------------- base/hmath.h | 2 +- base/htime.c | 36 +++++++++++++++++++++--------------- base/htime.h | 18 ++++++++++++++++++ base/queue.h | 13 +++++++++++-- 8 files changed, 87 insertions(+), 38 deletions(-) diff --git a/base/hbase.c b/base/hbase.c index 583161271..4837aa3ad 100644 --- a/base/hbase.c +++ b/base/hbase.c @@ -501,9 +501,17 @@ int hv_parse_url(hurl_t* stURL, const char* strURL) { stURL->fields[HV_URL_PORT].off = port + 1 - begin; stURL->fields[HV_URL_PORT].len = ep - port - 1; // atoi + unsigned int parsed_port = 0; for (unsigned short i = 1; i <= stURL->fields[HV_URL_PORT].len; ++i) { - stURL->port = stURL->port * 10 + (port[i] - '0'); + if (port[i] < '0' || port[i] > '9') { + return -2; + } + parsed_port = parsed_port * 10 + (port[i] - '0'); + if (parsed_port > 65535) { + return -3; + } } + stURL->port = (unsigned short)parsed_port; } else { port = ep; // set default port diff --git a/base/hbase.h b/base/hbase.h index c1ff0f46f..0423d4606 100644 --- a/base/hbase.h +++ b/base/hbase.h @@ -70,7 +70,7 @@ HV_EXPORT bool hv_wildcard_match(const char* str, const char* pattern); HV_EXPORT char* hv_strncpy(char* dest, const char* src, size_t n); // strncat n = sizeof(dest_buf)-1-strlen(dest) -// hv_strncpy n = sizeof(dest_buf) +// hv_strncat n = sizeof(dest_buf) HV_EXPORT char* hv_strncat(char* dest, const char* src, size_t n); #if !HAVE_STRLCPY diff --git a/base/hdef.h b/base/hdef.h index 0f79c75f0..4189bf090 100644 --- a/base/hdef.h +++ b/base/hdef.h @@ -167,7 +167,7 @@ #ifndef MAKE_FOURCC #define MAKE_FOURCC(a, b, c, d) \ -( ((uint32)d) | ( ((uint32)c) << 8 ) | ( ((uint32)b) << 16 ) | ( ((uint32)a) << 24 ) ) +( ((uint32_t)d) | ( ((uint32_t)c) << 8 ) | ( ((uint32_t)b) << 16 ) | ( ((uint32_t)a) << 24 ) ) #endif #ifndef MAX diff --git a/base/hlog.c b/base/hlog.c index 24540374c..ad426ea51 100644 --- a/base/hlog.c +++ b/base/hlog.c @@ -32,6 +32,20 @@ #define SECONDS_PER_WEEK 604800 // 7*24*3600; static int s_gmtoff = 28800; // 8*3600 +static void init_gmtoff() { + time_t ts = time(NULL); + struct tm* local_tm = localtime(&ts); + struct tm* gmt_tm = gmtime(&ts); + s_gmtoff = (local_tm->tm_hour - gmt_tm->tm_hour) * 3600 + + (local_tm->tm_min - gmt_tm->tm_min) * 60 + + (local_tm->tm_sec - gmt_tm->tm_sec); + + if (local_tm->tm_yday > gmt_tm->tm_yday) { + s_gmtoff += SECONDS_PER_DAY; + } else if (local_tm->tm_yday < gmt_tm->tm_yday) { + s_gmtoff -= SECONDS_PER_DAY; + } +} struct logger_s { logger_handler handler; @@ -79,13 +93,7 @@ static void logger_init(logger_t* logger) { } logger_t* logger_create() { - // init gmtoff here - time_t ts = time(NULL); - struct tm* local_tm = localtime(&ts); - int local_hour = local_tm->tm_hour; - struct tm* gmt_tm = gmtime(&ts); - int gmt_hour = gmt_tm->tm_hour; - s_gmtoff = (local_hour - gmt_hour) * SECONDS_PER_HOUR; + init_gmtoff(); logger_t* logger = (logger_t*)malloc(sizeof(logger_t)); logger_init(logger); @@ -375,17 +383,17 @@ int logger_print(logger_t* logger, int level, const char* fmt, ...) { us = tm.wMilliseconds * 1000; #else struct timeval tv; - struct tm* tm = NULL; gettimeofday(&tv, NULL); - time_t tt = tv.tv_sec; - struct tm tm_buf; - tm = localtime_r(&tt, &tm_buf); - year = tm->tm_year + 1900; - month = tm->tm_mon + 1; - day = tm->tm_mday; - hour = tm->tm_hour; - min = tm->tm_min; - sec = tm->tm_sec; + time_t ts = tv.tv_sec; + struct tm tm; + memset(&tm, 0, sizeof(tm)); + localtime_r(&ts, &tm); + year = tm.tm_year + 1900; + month = tm.tm_mon + 1; + day = tm.tm_mday; + hour = tm.tm_hour; + min = tm.tm_min; + sec = tm.tm_sec; us = tv.tv_usec; #endif diff --git a/base/hmath.h b/base/hmath.h index 6cff816f7..27570911a 100644 --- a/base/hmath.h +++ b/base/hmath.h @@ -92,7 +92,7 @@ static inline int asn1_encode(long long value, unsigned char* buf) { *p = (unsigned char)value; return 3; } - else if (value < 16777126) + else if (value < 16777216) { *p = 0x83; p++; diff --git a/base/htime.c b/base/htime.c index 8e053df81..a53a24a53 100644 --- a/base/htime.c +++ b/base/htime.c @@ -73,14 +73,17 @@ datetime_t datetime_now() { } datetime_t datetime_localtime(time_t seconds) { - struct tm* tm = localtime(&seconds); + struct tm tm; + memset(&tm, 0, sizeof(tm)); + hv_localtime_r(seconds, &tm); datetime_t dt; - dt.year = tm->tm_year + 1900; - dt.month = tm->tm_mon + 1; - dt.day = tm->tm_mday; - dt.hour = tm->tm_hour; - dt.min = tm->tm_min; - dt.sec = tm->tm_sec; + dt.year = tm.tm_year + 1900; + dt.month = tm.tm_mon + 1; + dt.day = tm.tm_mday; + dt.hour = tm.tm_hour; + dt.min = tm.tm_min; + dt.sec = tm.tm_sec; + dt.ms = 0; return dt; } @@ -88,8 +91,8 @@ time_t datetime_mktime(datetime_t* dt) { struct tm tm; time_t ts; time(&ts); - struct tm* ptm = localtime(&ts); - memcpy(&tm, ptm, sizeof(struct tm)); + memset(&tm, 0, sizeof(tm)); + hv_localtime_r(ts, &tm); tm.tm_year = dt->year - 1900; tm.tm_mon = dt->month - 1; tm.tm_mday = dt->day; @@ -171,12 +174,14 @@ char* datetime_fmt_iso(datetime_t* dt, char* buf) { } char* gmtime_fmt(time_t time, char* buf) { - struct tm* tm = gmtime(&time); - //strftime(buf, GMTIME_FMT_BUFLEN, "%a, %d %b %Y %H:%M:%S GMT", tm); + struct tm tm; + memset(&tm, 0, sizeof(tm)); + hv_gmtime_r(time, &tm); + //strftime(buf, GMTIME_FMT_BUFLEN, "%a, %d %b %Y %H:%M:%S GMT", &tm); sprintf(buf, GMTIME_FMT, - s_weekdays[tm->tm_wday], - tm->tm_mday, s_months[tm->tm_mon], tm->tm_year + 1900, - tm->tm_hour, tm->tm_min, tm->tm_sec); + s_weekdays[tm.tm_wday], + tm.tm_mday, s_months[tm.tm_mon], tm.tm_year + 1900, + tm.tm_hour, tm.tm_min, tm.tm_sec); return buf; } @@ -228,7 +233,8 @@ time_t cron_next_timeout(int minute, int hour, int day, int week, int month) { struct tm tm; time_t tt; time(&tt); - tm = *localtime(&tt); + memset(&tm, 0, sizeof(tm)); + hv_localtime_r(tt, &tm); time_t tt_round = 0; tm.tm_sec = 0; diff --git a/base/htime.h b/base/htime.h index 4d6dd016c..8eacecf8f 100644 --- a/base/htime.h +++ b/base/htime.h @@ -53,6 +53,24 @@ HV_INLINE int gettimeofday(struct timeval *tv, struct timezone *tz) { } #endif +HV_INLINE struct tm* hv_localtime_r(time_t ts, struct tm* tm) { +#ifdef OS_WIN + localtime_s(tm, &ts); +#else + tm = localtime_r(&ts, tm); +#endif + return tm; +} + +HV_INLINE struct tm* hv_gmtime_r(time_t ts, struct tm* tm) { +#ifdef OS_WIN + gmtime_s(tm, &ts); +#else + tm = gmtime_r(&ts, tm); +#endif + return tm; +} + HV_EXPORT unsigned int gettick_ms(); HV_INLINE unsigned long long gettimeofday_ms() { struct timeval tv; diff --git a/base/queue.h b/base/queue.h index 17140a9e8..960fdca03 100644 --- a/base/queue.h +++ b/base/queue.h @@ -70,6 +70,16 @@ static inline void qtype##_cleanup(qtype* p) {\ p->_offset = p->size = p->maxsize = 0;\ }\ \ +static inline void qtype##_realign(qtype* p) {\ + if (p->size == 0) {\ + p->_offset = 0;\ + }\ + else if (p->_offset > 0) {\ + memmove(p->ptr, p->ptr + p->_offset, sizeof(type) * p->size);\ + p->_offset = 0;\ + }\ +}\ +\ static inline void qtype##_resize(qtype* p, int maxsize) {\ if (maxsize == 0) maxsize = QUEUE_INIT_SIZE;\ p->ptr = (type*)hv_realloc(p->ptr, sizeof(type) * maxsize, sizeof(type) * p->maxsize);\ @@ -85,8 +95,7 @@ static inline void qtype##_push_back(qtype* p, type* elem) {\ qtype##_double_resize(p);\ }\ else if (p->_offset + p->size == p->maxsize) {\ - memmove(p->ptr, p->ptr + p->_offset, sizeof(type) * p->size);\ - p->_offset = 0;\ + qtype##_realign(p);\ }\ p->ptr[p->_offset + p->size] = *elem;\ p->size++;\ From 464de4086739120292416a767a55a77d43a577bb Mon Sep 17 00:00:00 2001 From: ithewei Date: Fri, 22 May 2026 14:20:04 +0800 Subject: [PATCH 2/9] fix: apply suggestions for cpputil dir from ai code review --- cpputil/LRUCache.h | 173 +++++++++++++++++---------------- cpputil/RAII.cpp | 17 +++- cpputil/ThreadLocalStorage.cpp | 12 ++- cpputil/hdir.cpp | 4 +- cpputil/hfile.h | 15 ++- cpputil/hobjectpool.h | 28 +++--- cpputil/hpath.cpp | 5 +- cpputil/hscope.h | 9 +- cpputil/hstring.cpp | 3 + cpputil/hthreadpool.h | 23 +++-- cpputil/hurl.cpp | 4 +- cpputil/ifconfig.cpp | 6 ++ cpputil/singleton.h | 11 ++- 13 files changed, 184 insertions(+), 126 deletions(-) diff --git a/cpputil/LRUCache.h b/cpputil/LRUCache.h index 8124aff1f..c24c32c1c 100755 --- a/cpputil/LRUCache.h +++ b/cpputil/LRUCache.h @@ -6,6 +6,7 @@ #include #include #include +#include namespace hv { @@ -92,23 +93,6 @@ class LRUCache { return true; } - /** - * @brief Get value by key (alternative interface) - * @param key The key to search for - * @return Pointer to value if exists, nullptr otherwise - */ - Value* get(const Key& key) { - std::lock_guard lock(mutex_); - auto it = hash_map_.find(key); - if (it == hash_map_.end()) { - return nullptr; - } - - // Move to front (most recently used) - move_to_front(it->second); - return &(it->second->value); - } - /** * @brief Put key-value pair into cache * @param key The key @@ -116,23 +100,29 @@ class LRUCache { * @return true if new item was added, false if existing item was updated */ bool put(const Key& key, const Value& value) { - std::lock_guard lock(mutex_); - auto it = hash_map_.find(key); - - if (it != hash_map_.end()) { - // Update existing item - it->second->value = value; - move_to_front(it->second); - return false; - } - - // Add new item - if (node_list_.size() >= capacity_) { - evict_lru(); + std::vector evicted_nodes; + eviction_callback_t callback; + { + std::lock_guard lock(mutex_); + auto it = hash_map_.find(key); + + if (it != hash_map_.end()) { + // Update existing item + it->second->value = value; + move_to_front(it->second); + return false; + } + + // Add new item + if (node_list_.size() >= capacity_) { + evict_lru(evicted_nodes); + callback = eviction_callback_; + } + + node_list_.emplace_front(key, value); + hash_map_[key] = node_list_.begin(); } - - node_list_.emplace_front(key, value); - hash_map_[key] = node_list_.begin(); + run_eviction_callbacks(callback, evicted_nodes); return true; } @@ -142,20 +132,24 @@ class LRUCache { * @return true if item was removed, false if key not found */ bool remove(const Key& key) { - std::lock_guard lock(mutex_); - auto it = hash_map_.find(key); - if (it == hash_map_.end()) { - return false; - } - - // Call eviction callback if set - if (eviction_callback_) { - eviction_callback_(it->second->key, it->second->value); + std::vector evicted_nodes; + eviction_callback_t callback; + bool removed = false; + { + std::lock_guard lock(mutex_); + auto it = hash_map_.find(key); + if (it == hash_map_.end()) { + return false; + } + + evicted_nodes.push_back(*(it->second)); + callback = eviction_callback_; + node_list_.erase(it->second); + hash_map_.erase(it); + removed = true; } - - node_list_.erase(it->second); - hash_map_.erase(it); - return true; + run_eviction_callbacks(callback, evicted_nodes); + return removed; } /** @@ -172,14 +166,16 @@ class LRUCache { * @brief Clear all items from cache */ void clear() { - std::lock_guard lock(mutex_); - if (eviction_callback_) { - for (const auto& node : node_list_) { - eviction_callback_(node.key, node.value); - } + std::vector evicted_nodes; + eviction_callback_t callback; + { + std::lock_guard lock(mutex_); + evicted_nodes.assign(node_list_.begin(), node_list_.end()); + callback = eviction_callback_; + node_list_.clear(); + hash_map_.clear(); } - node_list_.clear(); - hash_map_.clear(); + run_eviction_callbacks(callback, evicted_nodes); } /** @@ -216,14 +212,20 @@ class LRUCache { if (new_capacity == 0) { new_capacity = 1; // Minimum capacity of 1 } - - std::lock_guard lock(mutex_); - capacity_ = new_capacity; - - // Evict excess items if necessary - while (node_list_.size() > capacity_) { - evict_lru(); + + std::vector evicted_nodes; + eviction_callback_t callback; + { + std::lock_guard lock(mutex_); + capacity_ = new_capacity; + + // Evict excess items if necessary + while (node_list_.size() > capacity_) { + evict_lru(evicted_nodes); + } + callback = eviction_callback_; } + run_eviction_callbacks(callback, evicted_nodes); } /** @@ -247,25 +249,26 @@ class LRUCache { */ template size_t remove_if(Predicate predicate) { - std::lock_guard lock(mutex_); + std::vector evicted_nodes; + eviction_callback_t callback; size_t removed_count = 0; - - auto it = node_list_.begin(); - while (it != node_list_.end()) { - if (predicate(it->key, it->value)) { - // Call eviction callback if set - if (eviction_callback_) { - eviction_callback_(it->key, it->value); + { + std::lock_guard lock(mutex_); + + auto it = node_list_.begin(); + while (it != node_list_.end()) { + if (predicate(it->key, it->value)) { + evicted_nodes.push_back(*it); + hash_map_.erase(it->key); + it = node_list_.erase(it); + removed_count++; + } else { + ++it; } - - hash_map_.erase(it->key); - it = node_list_.erase(it); - removed_count++; - } else { - ++it; } + callback = eviction_callback_; } - + run_eviction_callbacks(callback, evicted_nodes); return removed_count; } @@ -280,21 +283,25 @@ class LRUCache { } } + void run_eviction_callbacks(const eviction_callback_t& callback, const std::vector& evicted_nodes) { + if (!callback) { + return; + } + for (const auto& node : evicted_nodes) { + callback(node.key, node.value); + } + } + /** * @brief Evict least recently used item */ - void evict_lru() { + void evict_lru(std::vector& evicted_nodes) { if (node_list_.empty()) { return; } - + auto last = std::prev(node_list_.end()); - - // Call eviction callback if set - if (eviction_callback_) { - eviction_callback_(last->key, last->value); - } - + evicted_nodes.push_back(*last); hash_map_.erase(last->key); node_list_.erase(last); } diff --git a/cpputil/RAII.cpp b/cpputil/RAII.cpp index 078380ad7..b522813a8 100644 --- a/cpputil/RAII.cpp +++ b/cpputil/RAII.cpp @@ -1,5 +1,7 @@ #include "hplatform.h" +#include + #ifdef OS_WIN #ifdef ENABLE_WINDUMP #include @@ -18,12 +20,23 @@ static LONG UnhandledException(EXCEPTION_POINTERS *pException) { modulefilename, st.wYear, st.wMonth, st.wDay, st.wHour, st.wMinute, st.wSecond, st.wMilliseconds); HANDLE hDumpFile = CreateFile(filename, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); + if (hDumpFile == INVALID_HANDLE_VALUE) { + DWORD err = GetLastError(); + fprintf(stderr, "CreateFile(%s) failed, error=%lu\n", filename, (unsigned long)err); + return EXCEPTION_EXECUTE_HANDLER; + } MINIDUMP_EXCEPTION_INFORMATION dumpInfo; dumpInfo.ExceptionPointers = pException; dumpInfo.ThreadId = GetCurrentThreadId(); dumpInfo.ClientPointers = TRUE; - MiniDumpWriteDump(GetCurrentProcess(), GetCurrentProcessId(), hDumpFile, MiniDumpNormal, &dumpInfo, NULL, NULL); - CloseHandle(hDumpFile); + if (!MiniDumpWriteDump(GetCurrentProcess(), GetCurrentProcessId(), hDumpFile, MiniDumpNormal, &dumpInfo, NULL, NULL)) { + DWORD err = GetLastError(); + fprintf(stderr, "MiniDumpWriteDump(%s) failed, error=%lu\n", filename, (unsigned long)err); + } + if (!CloseHandle(hDumpFile)) { + DWORD err = GetLastError(); + fprintf(stderr, "CloseHandle(%s) failed, error=%lu\n", filename, (unsigned long)err); + } return EXCEPTION_EXECUTE_HANDLER; } #endif diff --git a/cpputil/ThreadLocalStorage.cpp b/cpputil/ThreadLocalStorage.cpp index 8b72786e1..fce8c344b 100644 --- a/cpputil/ThreadLocalStorage.cpp +++ b/cpputil/ThreadLocalStorage.cpp @@ -4,13 +4,19 @@ namespace hv { +static inline bool tls_index_valid(int idx) { + return idx >= 0 && idx < ThreadLocalStorage::MAX_NUM; +} + ThreadLocalStorage ThreadLocalStorage::tls[ThreadLocalStorage::MAX_NUM]; void ThreadLocalStorage::set(int idx, void* val) { - return tls[idx].set(val); + if (!tls_index_valid(idx)) return; + tls[idx].set(val); } void* ThreadLocalStorage::get(int idx) { + if (!tls_index_valid(idx)) return NULL; return tls[idx].get(); } @@ -24,8 +30,8 @@ const char* ThreadLocalStorage::threadName() { return (char*)value; } - static char unnamed[32] = {0}; - snprintf(unnamed, sizeof(unnamed)-1, "thread-%ld", hv_gettid()); + thread_local char unnamed[32] = {0}; + snprintf(unnamed, sizeof(unnamed) - 1, "thread-%ld", hv_gettid()); return unnamed; } diff --git a/cpputil/hdir.cpp b/cpputil/hdir.cpp index a158e71e3..2137603e1 100644 --- a/cpputil/hdir.cpp +++ b/cpputil/hdir.cpp @@ -20,7 +20,7 @@ static bool less(const hdir_t& lhs, const hdir_t& rhs) { int listdir(const char* dir, std::list& dirs) { int dirlen = strlen(dir); - if (dirlen > 256) { + if (dirlen == 0 || dirlen > 256) { return -1; } char path[512]; @@ -64,7 +64,7 @@ int listdir(const char* dir, std::list& dirs) { strcat(path, "*"); WIN32_FIND_DATAW data; HANDLE h = FindFirstFileW(hv::utf8_to_wchar(path).c_str(), &data); - if (h == NULL) { + if (h == INVALID_HANDLE_VALUE) { return -1; } hdir_t tmp; diff --git a/cpputil/hfile.h b/cpputil/hfile.h index 0d681ea6a..9f8b5b9b8 100644 --- a/cpputil/hfile.h +++ b/cpputil/hfile.h @@ -42,7 +42,12 @@ class HFile { int rename(const char* newpath) { close(); - return ::rename(filepath, newpath); + int ret = ::rename(filepath, newpath); + if (ret == 0) { + strncpy(filepath, newpath, MAX_PATH - 1); + filepath[MAX_PATH - 1] = '\0'; + } + return ret; } size_t read(void* ptr, size_t len) { @@ -119,7 +124,15 @@ class HFile { int readrange(std::string& str, size_t from = 0, size_t to = 0) { size_t filesize = size(); if (filesize == 0) return 0; + if (from >= filesize) { + str.clear(); + return 0; + } if (to == 0 || to >= filesize) to = filesize - 1; + if (from > to) { + str.clear(); + return 0; + } size_t readbytes = to - from + 1; str.resize(readbytes); fseek(fp, from, SEEK_SET); diff --git a/cpputil/hobjectpool.h b/cpputil/hobjectpool.h index 31b9f6493..725f60e7e 100644 --- a/cpputil/hobjectpool.h +++ b/cpputil/hobjectpool.h @@ -9,6 +9,7 @@ #include #include #include +#include #define DEFAULT_OBJECT_POOL_INIT_NUM 0 #define DEFAULT_OBJECT_POOL_MAX_NUM 4 @@ -66,27 +67,20 @@ class HObjectPool { std::unique_lock locker(mutex_); if (_object_num < _max_num) { ++_object_num; - // NOTE: unlock to avoid TFactory::create block - mutex_.unlock(); + locker.unlock(); T* p = TFactory::create(); - mutex_.lock(); - if (!p) --_object_num; + locker.lock(); + if (!p) { + --_object_num; + } return std::shared_ptr(p); } - if (_timeout > 0) { - std::cv_status status = cond_.wait_for(locker, std::chrono::milliseconds(_timeout)); - if (status == std::cv_status::timeout) { - return NULL; - } - if (!objects_.empty()) { - pObj = objects_.front(); - objects_.pop_front(); - return pObj; - } - else { - // WARN: No idle object - } + if (_timeout > 0 && cond_.wait_for(locker, std::chrono::milliseconds(_timeout), [this]() { + return !objects_.empty(); + })) { + pObj = objects_.front(); + objects_.pop_front(); } return pObj; } diff --git a/cpputil/hpath.cpp b/cpputil/hpath.cpp index 0d552dd6d..e38929633 100644 --- a/cpputil/hpath.cpp +++ b/cpputil/hpath.cpp @@ -26,11 +26,9 @@ bool HPath::islink(const char* path) { #ifdef OS_WIN return HPath::isdir(path) && (GetFileAttributesA(path) & FILE_ATTRIBUTE_REPARSE_POINT); #else - if (access(path, 0) != 0) return false; struct stat st; memset(&st, 0, sizeof(st)); - lstat(path, &st); - return S_ISLNK(st.st_mode); + return (lstat(path, &st) == 0) && S_ISLNK(st.st_mode); #endif } @@ -97,6 +95,7 @@ std::string HPath::suffixname(const std::string& filepath) { } std::string HPath::join(const std::string& dir, const std::string& filename) { + if (dir.empty()) return filename; char separator = '/'; #ifdef OS_WIN if (dir.find_first_of("\\") != std::string::npos) { diff --git a/cpputil/hscope.h b/cpputil/hscope.h index 0222639f2..a723353cf 100644 --- a/cpputil/hscope.h +++ b/cpputil/hscope.h @@ -2,6 +2,7 @@ #define HV_SCOPE_H_ #include +#include typedef std::function Function; #include "hdef.h" @@ -23,8 +24,12 @@ class ScopeCleanup { _cleanup = std::bind(std::forward(fn), std::forward(args)...); } - ~ScopeCleanup() { - _cleanup(); + ~ScopeCleanup() noexcept { + if (!_cleanup) return; + try { + _cleanup(); + } catch (...) { + } } private: diff --git a/cpputil/hstring.cpp b/cpputil/hstring.cpp index a9b77f377..9728d8b9a 100644 --- a/cpputil/hstring.cpp +++ b/cpputil/hstring.cpp @@ -36,6 +36,7 @@ std::string& tolower(std::string& str) { std::string& reverse(std::string& str) { // std::reverse(str.begin(), str.end()); + if (str.length() < 2) return str; char* b = (char*)str.c_str(); char* e = b + str.length() - 1; char tmp; @@ -162,10 +163,12 @@ std::string ltrim(const std::string& str, const char* chars) { std::string rtrim(const std::string& str, const char* chars) { std::string::size_type pos = str.find_last_not_of(chars); + if (pos == std::string::npos) return ""; return str.substr(0, pos+1); } std::string trim_pairs(const std::string& str, const char* pairs) { + if (str.size() < 2) return str; const char* s = str.c_str(); const char* e = str.c_str() + str.size() - 1; const char* p = pairs; diff --git a/cpputil/hthreadpool.h b/cpputil/hthreadpool.h index 8df987178..a2cfb5049 100644 --- a/cpputil/hthreadpool.h +++ b/cpputil/hthreadpool.h @@ -35,6 +35,7 @@ class HThreadPool { , status(STOP) , cur_thread_num(0) , idle_thread_num(0) + , active_task_num(0) {} virtual ~HThreadPool() { @@ -82,6 +83,7 @@ class HThreadPool { if (status == STOP) return -1; status = STOP; task_cond.notify_all(); + wait_cond.notify_all(); for (auto& i : threads) { if (i.thread->joinable()) { i.thread->join(); @@ -90,6 +92,7 @@ class HThreadPool { threads.clear(); cur_thread_num = 0; idle_thread_num = 0; + active_task_num = 0; return 0; } @@ -108,12 +111,10 @@ class HThreadPool { } int wait() { - while (status != STOP) { - if (tasks.empty() && idle_thread_num == cur_thread_num) { - break; - } - std::this_thread::yield(); - } + std::unique_lock locker(task_mutex); + wait_cond.wait(locker, [this]() { + return status == STOP || (tasks.empty() && active_task_num == 0); + }); return 0; } @@ -127,7 +128,7 @@ class HThreadPool { template auto commit(Fn&& fn, Args&&... args) -> std::future { if (status == STOP) start(); - if (idle_thread_num <= tasks.size() && cur_thread_num < max_thread_num) { + if (idle_thread_num <= taskNum() && cur_thread_num < max_thread_num) { createThread(); } using RetType = decltype(fn(args...)); @@ -169,12 +170,18 @@ class HThreadPool { continue; } --idle_thread_num; + ++active_task_num; task = std::move(tasks.front()); tasks.pop(); } if (task) { task(); + std::lock_guard locker(task_mutex); + --active_task_num; ++idle_thread_num; + if (tasks.empty() && active_task_num == 0) { + wait_cond.notify_all(); + } } } }); @@ -244,6 +251,8 @@ class HThreadPool { std::queue tasks; std::mutex task_mutex; std::condition_variable task_cond; + std::condition_variable wait_cond; + std::atomic active_task_num; }; #endif // HV_THREAD_POOL_H_ diff --git a/cpputil/hurl.cpp b/cpputil/hurl.cpp index c3f9024dc..79f6d6262 100644 --- a/cpputil/hurl.cpp +++ b/cpputil/hurl.cpp @@ -69,8 +69,10 @@ std::string HUrl::escape(const std::string& str, const char* unescaped_chars) { std::string HUrl::unescape(const std::string& str) { std::string ostr; const char* p = str.c_str(); - while (*p != '\0') { + const char* end = p + str.size(); + while (p < end) { if (*p == '%' && + end - p >= 3 && IS_HEX(p[1]) && IS_HEX(p[2])) { ostr += ((hex2i(p[1]) << 4) | hex2i(p[2])); diff --git a/cpputil/ifconfig.cpp b/cpputil/ifconfig.cpp index cfdc8924e..6aaeec8b9 100644 --- a/cpputil/ifconfig.cpp +++ b/cpputil/ifconfig.cpp @@ -173,6 +173,9 @@ int ifconfig(std::vector& ifcs) { if (ret != 0) return ret; ifconfig_s tmp; for (ifap = ifas; ifap != NULL; ifap = ifap->ifa_next) { + if (ifap->ifa_addr == NULL) { + continue; + } if (ifap->ifa_addr->sa_family == AF_INET) { // ipv4 struct sockaddr_in* addr = (struct sockaddr_in*)ifap->ifa_addr; @@ -198,6 +201,9 @@ int ifconfig(std::vector& ifcs) { } for (ifap = ifas; ifap != NULL; ifap = ifap->ifa_next) { + if (ifap->ifa_addr == NULL) { + continue; + } if (ifap->ifa_addr->sa_family == AF_LINK) { // hwaddr for (auto iter = ifcs.begin(); iter != ifcs.end(); ++iter) { diff --git a/cpputil/singleton.h b/cpputil/singleton.h index ff78af616..58b21c480 100644 --- a/cpputil/singleton.h +++ b/cpputil/singleton.h @@ -14,22 +14,23 @@ private: \ DISABLE_COPY(Class) \ static Class* s_pInstance; \ - static std::once_flag s_initFlag; \ static std::mutex s_mutex; #define SINGLETON_IMPL(Class) \ Class* Class::s_pInstance = NULL; \ - std::once_flag Class::s_initFlag; \ std::mutex Class::s_mutex; \ Class* Class::instance() { \ - std::call_once(s_initFlag, []() {s_pInstance = new Class;}); \ - return s_pInstance; \ + std::lock_guard lock(s_mutex); \ + if (s_pInstance == NULL) { \ + s_pInstance = new Class; \ + } \ + return s_pInstance; \ } \ void Class::exitInstance() { \ std::lock_guard lock(s_mutex); \ if (s_pInstance) { \ delete s_pInstance; \ - s_pInstance = nullptr; \ + s_pInstance = NULL; \ } \ } From 7e3b9d1d2759188555a50ee91f9fe17df0624abe Mon Sep 17 00:00:00 2001 From: ithewei Date: Fri, 22 May 2026 22:31:19 +0800 Subject: [PATCH 3/9] fix: apply suggestions for ssl dir from ai code review --- ssl/gnutls.c | 41 +++++++++++++++++++++++++++++++---------- ssl/mbedtls.c | 22 +++++++++++++++++----- ssl/openssl.c | 7 +++++-- 3 files changed, 53 insertions(+), 17 deletions(-) diff --git a/ssl/gnutls.c b/ssl/gnutls.c index 465c89ad8..ccdf1ba0b 100644 --- a/ssl/gnutls.c +++ b/ssl/gnutls.c @@ -101,10 +101,23 @@ static int hssl_init(hssl_t ssl, int endpoint) { if (ssl == NULL) return HSSL_ERROR; gnutls_t* gnutls = (gnutls_t*)ssl; if (gnutls->session == NULL) { - gnutls_init(&gnutls->session, endpoint); - gnutls_priority_set_direct(gnutls->session, "NORMAL", NULL); - gnutls_credentials_set(gnutls->session, GNUTLS_CRD_CERTIFICATE, gnutls->ctx); - gnutls_transport_set_ptr(gnutls->session, (gnutls_transport_ptr_t)(ptrdiff_t)gnutls->fd); + gnutls_session_t session = NULL; + int ret = gnutls_init(&session, endpoint); + if (ret != GNUTLS_E_SUCCESS) { + return HSSL_ERROR; + } + ret = gnutls_priority_set_direct(session, "NORMAL", NULL); + if (ret != GNUTLS_E_SUCCESS) { + gnutls_deinit(session); + return HSSL_ERROR; + } + ret = gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, gnutls->ctx); + if (ret != GNUTLS_E_SUCCESS) { + gnutls_deinit(session); + return HSSL_ERROR; + } + gnutls_transport_set_ptr(session, (gnutls_transport_ptr_t)(ptrdiff_t)gnutls->fd); + gnutls->session = session; } return HSSL_OK; } @@ -144,7 +157,9 @@ int hssl_accept(hssl_t ssl) { if (ssl == NULL) return HSSL_ERROR; gnutls_t* gnutls = (gnutls_t*)ssl; if (gnutls->session == NULL) { - hssl_init(ssl, GNUTLS_SERVER); + if (hssl_init(ssl, GNUTLS_SERVER) != HSSL_OK) { + return HSSL_ERROR; + } } return hssl_handshake(ssl); } @@ -153,7 +168,9 @@ int hssl_connect(hssl_t ssl) { if (ssl == NULL) return HSSL_ERROR; gnutls_t* gnutls = (gnutls_t*)ssl; if (gnutls->session == NULL) { - hssl_init(ssl, GNUTLS_CLIENT); + if (hssl_init(ssl, GNUTLS_CLIENT) != HSSL_OK) { + return HSSL_ERROR; + } } return hssl_handshake(ssl); } @@ -185,13 +202,17 @@ int hssl_close(hssl_t ssl) { } int hssl_set_sni_hostname(hssl_t ssl, const char* hostname) { - if (ssl == NULL) return HSSL_ERROR; + if (ssl == NULL || hostname == NULL) return HSSL_ERROR; gnutls_t* gnutls = (gnutls_t*)ssl; if (gnutls->session == NULL) { - hssl_init(ssl, GNUTLS_CLIENT); + if (hssl_init(ssl, GNUTLS_CLIENT) != HSSL_OK) { + return HSSL_ERROR; + } + } + if (gnutls_server_name_set(gnutls->session, GNUTLS_NAME_DNS, hostname, strlen(hostname)) != GNUTLS_E_SUCCESS) { + return HSSL_ERROR; } - gnutls_server_name_set(gnutls->session, GNUTLS_NAME_DNS, hostname, strlen(hostname)); - return 0; + return HSSL_OK; } #endif // WITH_GNUTLS diff --git a/ssl/mbedtls.c b/ssl/mbedtls.c index 4a14ead68..ff047d199 100644 --- a/ssl/mbedtls.c +++ b/ssl/mbedtls.c @@ -78,7 +78,10 @@ hssl_ctx_t hssl_ctx_new(hssl_ctx_opt_t* param) { endpoint = MBEDTLS_SSL_IS_SERVER; } } - mbedtls_ctr_drbg_seed(&ctx->ctr_drbg, mbedtls_entropy_func, &ctx->entropy, NULL, 0); + if (mbedtls_ctr_drbg_seed(&ctx->ctr_drbg, mbedtls_entropy_func, &ctx->entropy, NULL, 0) != 0) { + fprintf(stderr, "ssl ctr_drbg_seed failed!\n"); + goto error; + } if (mbedtls_ssl_config_defaults(&ctx->conf, endpoint, MBEDTLS_SSL_TRANSPORT_STREAM, MBEDTLS_SSL_PRESET_DEFAULT) != 0) { fprintf(stderr, "ssl config error!\n"); @@ -100,7 +103,7 @@ hssl_ctx_t hssl_ctx_new(hssl_ctx_opt_t* param) { } return ctx; error: - free(ctx); + hssl_ctx_free(ctx); return NULL; } @@ -136,10 +139,15 @@ static int __mbedtls_net_recv(void *ctx, unsigned char *buf, size_t len) { hssl_t hssl_new(hssl_ctx_t ssl_ctx, int fd) { struct mbedtls_ctx* mctx = (struct mbedtls_ctx*)ssl_ctx; + if (mctx == NULL) return NULL; mbedtls_ssl_context* ssl = (mbedtls_ssl_context*)malloc(sizeof(mbedtls_ssl_context)); if (ssl == NULL) return NULL; mbedtls_ssl_init(ssl); - mbedtls_ssl_setup(ssl, &mctx->conf); + if (mbedtls_ssl_setup(ssl, &mctx->conf) != 0) { + mbedtls_ssl_free(ssl); + free(ssl); + return NULL; + } mbedtls_ssl_set_bio(ssl, (void*)(intptr_t)fd, __mbedtls_net_send, __mbedtls_net_recv, NULL); return ssl; } @@ -147,6 +155,7 @@ hssl_t hssl_new(hssl_ctx_t ssl_ctx, int fd) { void hssl_free(hssl_t ssl) { if (ssl) { mbedtls_ssl_free(ssl); + free(ssl); ssl = NULL; } } @@ -185,10 +194,13 @@ int hssl_close(hssl_t ssl) { } int hssl_set_sni_hostname(hssl_t ssl, const char* hostname) { + if (ssl == NULL || hostname == NULL) return HSSL_ERROR; #ifdef MBEDTLS_X509_CRT_PARSE_C - mbedtls_ssl_set_hostname(ssl, hostname); + if (mbedtls_ssl_set_hostname(ssl, hostname) != 0) { + return HSSL_ERROR; + } #endif - return 0; + return HSSL_OK; } #endif // WITH_MBEDTLS diff --git a/ssl/openssl.c b/ssl/openssl.c index 0a0e27f01..ba6dbc014 100644 --- a/ssl/openssl.c +++ b/ssl/openssl.c @@ -160,10 +160,13 @@ int hssl_close(hssl_t ssl) { } int hssl_set_sni_hostname(hssl_t ssl, const char* hostname) { + if (ssl == NULL || hostname == NULL) return HSSL_ERROR; #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME - SSL_set_tlsext_host_name((SSL*)ssl, hostname); + if (SSL_set_tlsext_host_name((SSL*)ssl, hostname) != 1) { + return HSSL_ERROR; + } #endif - return 0; + return HSSL_OK; } #ifdef TLSEXT_TYPE_application_layer_protocol_negotiation From 7bb4fdc9d73dd0fb161dfe375feaee24244e4fa8 Mon Sep 17 00:00:00 2001 From: ithewei Date: Fri, 22 May 2026 22:34:23 +0800 Subject: [PATCH 4/9] fix: apply suggestions for event dir from ai code review --- event/epoll.c | 20 ++++++++++++---- event/evport.c | 33 +++++++++++++++++--------- event/hloop.c | 37 ++++++++++++++++++----------- event/io_uring.c | 2 ++ event/iocp.c | 8 ++++++- event/kqueue.c | 61 ++++++++++++++++++++++++++++++++++-------------- event/nio.c | 26 +++++++++++++++++++-- event/select.c | 3 +-- event/unpack.c | 19 ++++++++++++++- 9 files changed, 156 insertions(+), 53 deletions(-) diff --git a/event/epoll.c b/event/epoll.c index 5fc6871e9..447b27aa0 100644 --- a/event/epoll.c +++ b/event/epoll.c @@ -4,6 +4,7 @@ #include "hplatform.h" #include "hdef.h" #include "hevent.h" +#include "hlog.h" #ifdef OS_WIN #include "wepoll/wepoll.h" @@ -25,9 +26,14 @@ typedef struct epoll_ctx_s { int iowatcher_init(hloop_t* loop) { if (loop->iowatcher) return 0; + epoll_handle_t epfd = epoll_create(EVENTS_INIT_SIZE); + if (epfd < 0) { + hloge("epoll_create failed: %d", socket_errno()); + return -1; + } epoll_ctx_t* epoll_ctx; HV_ALLOC_SIZEOF(epoll_ctx); - epoll_ctx->epfd = epoll_create(EVENTS_INIT_SIZE); + epoll_ctx->epfd = epfd; events_init(&epoll_ctx->events, EVENTS_INIT_SIZE); loop->iowatcher = epoll_ctx; return 0; @@ -44,7 +50,9 @@ int iowatcher_cleanup(hloop_t* loop) { int iowatcher_add_event(hloop_t* loop, int fd, int events) { if (loop->iowatcher == NULL) { - iowatcher_init(loop); + if (iowatcher_init(loop) != 0) { + return -1; + } } epoll_ctx_t* epoll_ctx = (epoll_ctx_t*)loop->iowatcher; hio_t* io = loop->ios.ptr[fd]; @@ -67,7 +75,9 @@ int iowatcher_add_event(hloop_t* loop, int fd, int events) { ee.events |= EPOLLOUT; } int op = io->events == 0 ? EPOLL_CTL_ADD : EPOLL_CTL_MOD; - epoll_ctl(epoll_ctx->epfd, op, fd, &ee); + if (epoll_ctl(epoll_ctx->epfd, op, fd, &ee) != 0) { + return -1; + } if (op == EPOLL_CTL_ADD) { if (epoll_ctx->events.size == epoll_ctx->events.maxsize) { events_double_resize(&epoll_ctx->events); @@ -100,7 +110,9 @@ int iowatcher_del_event(hloop_t* loop, int fd, int events) { ee.events &= ~EPOLLOUT; } int op = ee.events == 0 ? EPOLL_CTL_DEL : EPOLL_CTL_MOD; - epoll_ctl(epoll_ctx->epfd, op, fd, &ee); + if (epoll_ctl(epoll_ctx->epfd, op, fd, &ee) != 0) { + return -1; + } if (op == EPOLL_CTL_DEL) { epoll_ctx->events.size--; } diff --git a/event/evport.c b/event/evport.c index 8976dec74..215004988 100644 --- a/event/evport.c +++ b/event/evport.c @@ -5,6 +5,7 @@ #include "hplatform.h" #include "hdef.h" #include "hevent.h" +#include "hlog.h" #include @@ -26,9 +27,14 @@ static void evport_ctx_resize(evport_ctx_t* evport_ctx, int size) { int iowatcher_init(hloop_t* loop) { if (loop->iowatcher) return 0; + int port = port_create(); + if (port < 0) { + hloge("port_create failed: %d", errno); + return -1; + } evport_ctx_t* evport_ctx; HV_ALLOC_SIZEOF(evport_ctx); - evport_ctx->port = port_create(); + evport_ctx->port = port; evport_ctx->capacity = EVENTS_INIT_SIZE; evport_ctx->nevents = 0; int bytes = sizeof(port_event_t) * evport_ctx->capacity; @@ -48,7 +54,9 @@ int iowatcher_cleanup(hloop_t* loop) { int iowatcher_add_event(hloop_t* loop, int fd, int events) { if (loop->iowatcher == NULL) { - iowatcher_init(loop); + if (iowatcher_init(loop) != 0) { + return -1; + } } evport_ctx_t* evport_ctx = (evport_ctx_t*)loop->iowatcher; hio_t* io = loop->ios.ptr[fd]; @@ -115,20 +123,23 @@ int iowatcher_poll_events(hloop_t* loop, int timeout) { tp = &ts; } unsigned nevents = 1; - port_getn(evport_ctx->port, evport_ctx->events, evport_ctx->capacity, &nevents, tp); + if (port_getn(evport_ctx->port, evport_ctx->events, evport_ctx->capacity, &nevents, tp) != 0) { + return -1; + } for (int i = 0; i < nevents; ++i) { int fd = evport_ctx->events[i].portev_object; int revents = evport_ctx->events[i].portev_events; hio_t* io = loop->ios.ptr[fd]; - if (io) { - if (revents & POLLIN) { - io->revents |= HV_READ; - } - if (revents & POLLOUT) { - io->revents |= HV_WRITE; - } - EVENT_PENDING(io); + if (io == NULL) { + continue; + } + if (revents & POLLIN) { + io->revents |= HV_READ; + } + if (revents & POLLOUT) { + io->revents |= HV_WRITE; } + EVENT_PENDING(io); // Upon retrieval, the event object is no longer associated with the port. iowatcher_add_event(loop, fd, io->events); } diff --git a/event/hloop.c b/event/hloop.c index 5999daf90..6b1fcab83 100644 --- a/event/hloop.c +++ b/event/hloop.c @@ -101,7 +101,7 @@ static int hloop_process_ios(hloop_t* loop, int timeout) { // That is to call IO multiplexing function such as select, poll, epoll, etc. int nevents = iowatcher_poll_events(loop, timeout); if (nevents < 0) { - hlogd("poll_events error=%d", -nevents); + hlogd("iowatcher_poll_events failed err=%d", -nevents); } return nevents < 0 ? 0 : nevents; } @@ -814,6 +814,7 @@ void hio_detach(hio_t* io) { void hio_attach(hloop_t* loop, hio_t* io) { int fd = io->fd; + bool use_loop_readbuf = io->loop == NULL || hio_is_loop_readbuf(io); // NOTE: hio was not freed for reused when closed, but attached hio can't be reused, // so we need to free it if fd exists to avoid memory leak. hio_t* preio = __hio_get(loop, fd); @@ -822,8 +823,9 @@ void hio_attach(hloop_t* loop, hio_t* io) { } io->loop = loop; - // NOTE: use new_loop readbuf - hio_use_loop_readbuf(io); + if (use_loop_readbuf) { + hio_use_loop_readbuf(io); + } loop->ios.ptr[fd] = io; } @@ -841,11 +843,6 @@ int hio_add(hio_t* io, hio_cb cb, int events) { if (io->fd < 3) return -1; #endif hloop_t* loop = io->loop; - if (!io->active) { - EVENT_ADD(loop, io, cb); - loop->nios++; - } - if (!io->ready) { hio_ready(io); } @@ -854,9 +851,17 @@ int hio_add(hio_t* io, hio_cb cb, int events) { io->cb = (hevent_cb)cb; } - if (!(io->events & events)) { - iowatcher_add_event(loop, io->fd, events); - io->events |= events; + int add_events = events & ~io->events; + if (add_events && iowatcher_add_event(loop, io->fd, add_events) != 0) { + hlogd("iowatcher_add_event failed fd=%d add_events=%d io->events=%d err=%d", + io->fd, add_events, io->events, socket_errno()); + return -1; + } + io->events |= add_events; + + if (!io->active) { + EVENT_ADD(loop, io, cb); + loop->nios++; } return 0; } @@ -869,10 +874,14 @@ int hio_del(hio_t* io, int events) { #endif if (!io->active) return -1; - if (io->events & events) { - iowatcher_del_event(io->loop, io->fd, events); - io->events &= ~events; + int del_events = io->events & events; + if (del_events && iowatcher_del_event(io->loop, io->fd, del_events) != 0) { + hlogd("iowatcher_del_event failed fd=%d del_events=%d io->events=%d err=%d", + io->fd, del_events, io->events, socket_errno()); + return -1; } + io->events &= ~del_events; + if (io->events == 0) { io->loop->nios--; // NOTE: not EVENT_DEL, avoid free diff --git a/event/io_uring.c b/event/io_uring.c index 0b677d126..baabd4db9 100644 --- a/event/io_uring.c +++ b/event/io_uring.c @@ -4,6 +4,7 @@ #include "hplatform.h" #include "hdef.h" #include "hevent.h" +#include "hlog.h" #include #include @@ -22,6 +23,7 @@ int iowatcher_init(hloop_t* loop) { HV_ALLOC_SIZEOF(ctx); int ret = io_uring_queue_init(IO_URING_ENTRIES, &ctx->ring, 0); if (ret < 0) { + hloge("io_uring_queue_init failed: %d", -ret); HV_FREE(ctx); return ret; } diff --git a/event/iocp.c b/event/iocp.c index caef1cfd4..5463bb96d 100644 --- a/event/iocp.c +++ b/event/iocp.c @@ -3,6 +3,7 @@ #ifdef EVENT_IOCP #include "hplatform.h" #include "hdef.h" +#include "hlog.h" #include "hevent.h" #include "overlapio.h" @@ -13,9 +14,14 @@ typedef struct iocp_ctx_s { int iowatcher_init(hloop_t* loop) { if (loop->iowatcher) return 0; + HANDLE iocp = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 0); + if (iocp == NULL) { + hloge("CreateIoCompletionPort failed: %d", GetLastError()); + return -1; + } iocp_ctx_t* iocp_ctx; HV_ALLOC_SIZEOF(iocp_ctx); - iocp_ctx->iocp = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 0); + iocp_ctx->iocp = iocp; loop->iowatcher = iocp_ctx; return 0; } diff --git a/event/kqueue.c b/event/kqueue.c index 5416280a1..ff927b9a9 100644 --- a/event/kqueue.c +++ b/event/kqueue.c @@ -3,6 +3,7 @@ #ifdef EVENT_KQUEUE #include "hplatform.h" #include "hdef.h" +#include "hlog.h" #include @@ -33,9 +34,14 @@ static void kqueue_ctx_resize(kqueue_ctx_t* kqueue_ctx, int size) { int iowatcher_init(hloop_t* loop) { if (loop->iowatcher) return 0; + int kqfd = kqueue(); + if (kqfd < 0) { + hloge("kqueue failed: %d", errno); + return -1; + } kqueue_ctx_t* kqueue_ctx; HV_ALLOC_SIZEOF(kqueue_ctx); - kqueue_ctx->kqfd = kqueue(); + kqueue_ctx->kqfd = kqfd; kqueue_ctx->capacity = EVENTS_INIT_SIZE; kqueue_ctx->nchanges = 0; int bytes = sizeof(struct kevent) * kqueue_ctx->capacity; @@ -57,7 +63,9 @@ int iowatcher_cleanup(hloop_t* loop) { static int __add_event(hloop_t* loop, int fd, int event) { if (loop->iowatcher == NULL) { - iowatcher_init(loop); + if (iowatcher_init(loop) != 0) { + return -1; + } } kqueue_ctx_t* kqueue_ctx = (kqueue_ctx_t*)loop->iowatcher; hio_t* io = loop->ios.ptr[fd]; @@ -77,16 +85,26 @@ static int __add_event(hloop_t* loop, int fd, int event) { struct timespec ts; ts.tv_sec = 0; ts.tv_nsec = 0; - kevent(kqueue_ctx->kqfd, kqueue_ctx->changes, kqueue_ctx->nchanges, NULL, 0, &ts); + if (kevent(kqueue_ctx->kqfd, kqueue_ctx->changes, kqueue_ctx->nchanges, NULL, 0, &ts) != 0) { + if (idx == kqueue_ctx->nchanges - 1) { + io->event_index[EVENT_INDEX(event)] = -1; + kqueue_ctx->nchanges--; + } + return -1; + } return 0; } int iowatcher_add_event(hloop_t* loop, int fd, int events) { if (events & HV_READ) { - __add_event(loop, fd, EVFILT_READ); + if (__add_event(loop, fd, EVFILT_READ) != 0) { + return -1; + } } if (events & HV_WRITE) { - __add_event(loop, fd, EVFILT_WRITE); + if (__add_event(loop, fd, EVFILT_WRITE) != 0) { + return -1; + } } return 0; } @@ -98,34 +116,38 @@ static int __del_event(hloop_t* loop, int fd, int event) { int idx = io->event_index[EVENT_INDEX(event)]; if (idx < 0) return 0; assert(kqueue_ctx->changes[idx].ident == fd); + struct kevent old_event = kqueue_ctx->changes[idx]; kqueue_ctx->changes[idx].flags = EV_DELETE; + struct timespec ts; + ts.tv_sec = 0; + ts.tv_nsec = 0; + if (kevent(kqueue_ctx->kqfd, kqueue_ctx->changes, kqueue_ctx->nchanges, NULL, 0, &ts) != 0) { + kqueue_ctx->changes[idx] = old_event; + return -1; + } io->event_index[EVENT_INDEX(event)] = -1; int lastidx = kqueue_ctx->nchanges - 1; if (idx < lastidx) { - // swap - struct kevent tmp; - tmp = kqueue_ctx->changes[idx]; kqueue_ctx->changes[idx] = kqueue_ctx->changes[lastidx]; - kqueue_ctx->changes[lastidx] = tmp; hio_t* last = loop->ios.ptr[kqueue_ctx->changes[idx].ident]; if (last) { last->event_index[EVENT_INDEX(kqueue_ctx->changes[idx].filter)] = idx; } } - struct timespec ts; - ts.tv_sec = 0; - ts.tv_nsec = 0; - kevent(kqueue_ctx->kqfd, kqueue_ctx->changes, kqueue_ctx->nchanges, NULL, 0, &ts); kqueue_ctx->nchanges--; return 0; } int iowatcher_del_event(hloop_t* loop, int fd, int events) { if (events & HV_READ) { - __del_event(loop, fd, EVFILT_READ); + if (__del_event(loop, fd, EVFILT_READ) != 0) { + return -1; + } } if (events & HV_WRITE) { - __del_event(loop, fd, EVFILT_WRITE); + if (__del_event(loop, fd, EVFILT_WRITE) != 0) { + return -1; + } } return 0; } @@ -145,6 +167,9 @@ int iowatcher_poll_events(hloop_t* loop, int timeout) { } int nkqueue = kevent(kqueue_ctx->kqfd, kqueue_ctx->changes, kqueue_ctx->nchanges, kqueue_ctx->events, kqueue_ctx->nchanges, tp); if (nkqueue < 0) { + if (errno == EINTR) { + return 0; + } perror("kevent"); return nkqueue; } @@ -156,13 +181,13 @@ int iowatcher_poll_events(hloop_t* loop, int timeout) { } ++nevents; int fd = kqueue_ctx->events[i].ident; - int revents = kqueue_ctx->events[i].filter; + int filter = kqueue_ctx->events[i].filter; hio_t* io = loop->ios.ptr[fd]; if (io) { - if (revents & EVFILT_READ) { + if (filter == EVFILT_READ) { io->revents |= HV_READ; } - if (revents & EVFILT_WRITE) { + if (filter == EVFILT_WRITE) { io->revents |= HV_WRITE; } EVENT_PENDING(io); diff --git a/event/nio.c b/event/nio.c index 337299ac0..4980237f4 100644 --- a/event/nio.c +++ b/event/nio.c @@ -70,15 +70,26 @@ static void ssl_server_handshake(hio_t* io) { int ret = hssl_accept(io->ssl); if (ret == 0) { // handshake finish - hio_del(io, HV_READ); + hio_del(io, HV_RDWR); printd("ssl handshake finished.\n"); __accept_cb(io); } else if (ret == HSSL_WANT_READ) { + if (io->events & HV_WRITE) { + hio_del(io, HV_WRITE); + } if ((io->events & HV_READ) == 0) { hio_add(io, ssl_server_handshake, HV_READ); } } + else if (ret == HSSL_WANT_WRITE) { + if (io->events & HV_READ) { + hio_del(io, HV_READ); + } + if ((io->events & HV_WRITE) == 0) { + hio_add(io, ssl_server_handshake, HV_WRITE); + } + } else { hloge("ssl server handshake failed: %d", ret); io->error = ERR_SSL_HANDSHAKE; @@ -91,15 +102,26 @@ static void ssl_client_handshake(hio_t* io) { int ret = hssl_connect(io->ssl); if (ret == 0) { // handshake finish - hio_del(io, HV_READ); + hio_del(io, HV_RDWR); printd("ssl handshake finished.\n"); __connect_cb(io); } else if (ret == HSSL_WANT_READ) { + if (io->events & HV_WRITE) { + hio_del(io, HV_WRITE); + } if ((io->events & HV_READ) == 0) { hio_add(io, ssl_client_handshake, HV_READ); } } + else if (ret == HSSL_WANT_WRITE) { + if (io->events & HV_READ) { + hio_del(io, HV_READ); + } + if ((io->events & HV_WRITE) == 0) { + hio_add(io, ssl_client_handshake, HV_WRITE); + } + } else { hloge("ssl client handshake failed: %d", ret); io->error = ERR_SSL_HANDSHAKE; diff --git a/event/select.c b/event/select.c index 8a35ea1eb..db57555fc 100644 --- a/event/select.c +++ b/event/select.c @@ -148,14 +148,13 @@ int iowatcher_poll_events(hloop_t* loop, int timeout) { for (int fd = 0; fd <= max_fd; ++fd) { revents = 0; if (FD_ISSET(fd, &readfds)) { - ++nevents; revents |= HV_READ; } if (FD_ISSET(fd, &writefds)) { - ++nevents; revents |= HV_WRITE; } if (revents) { + ++nevents; hio_t* io = loop->ios.ptr[fd]; if (io) { io->revents = revents; diff --git a/event/unpack.c b/event/unpack.c index 604b53c91..0c75b2a1f 100644 --- a/event/unpack.c +++ b/event/unpack.c @@ -25,7 +25,12 @@ int hio_unpack_by_fixed_length(hio_t* io, void* buf, int readbytes) { unpack_setting_t* setting = io->unpack_setting; int fixed_length = setting->fixed_length; - assert(io->readbuf.len >= fixed_length); + if (fixed_length <= 0 || io->readbuf.len < fixed_length) { + hloge("invalid fixed_length: %d", fixed_length); + io->error = ERR_INVALID_PARAM; + hio_close(io); + return -1; + } const unsigned char* p = sp; int remain = ep - p; @@ -158,6 +163,18 @@ int hio_unpack_by_length_field(hio_t* io, void* buf, int readbytes) { return -1; } package_len = head_len + body_len + setting->length_adjustment; + if (package_len < head_len || package_len == 0) { + hloge("invalid package length: %u", package_len); + io->error = ERR_INVALID_PARAM; + hio_close(io); + return -1; + } + if (package_len > setting->package_max_length) { + hloge("package length over %d bytes!", (int)setting->package_max_length); + io->error = ERR_OVER_LIMIT; + hio_close(io); + return -1; + } if (remain >= package_len) { hio_read_cb(io, (void*)p, package_len); handled += package_len; From 02d9919b5ddf289c7e22a7705dce91bf3d202ffe Mon Sep 17 00:00:00 2001 From: ithewei Date: Fri, 22 May 2026 22:43:44 +0800 Subject: [PATCH 5/9] fix: build error --- event/epoll.c | 1 + event/io_uring.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/event/epoll.c b/event/epoll.c index 447b27aa0..b7b28214b 100644 --- a/event/epoll.c +++ b/event/epoll.c @@ -5,6 +5,7 @@ #include "hdef.h" #include "hevent.h" #include "hlog.h" +#include "hsocket.h" #ifdef OS_WIN #include "wepoll/wepoll.h" diff --git a/event/io_uring.c b/event/io_uring.c index baabd4db9..ffd939653 100644 --- a/event/io_uring.c +++ b/event/io_uring.c @@ -23,7 +23,7 @@ int iowatcher_init(hloop_t* loop) { HV_ALLOC_SIZEOF(ctx); int ret = io_uring_queue_init(IO_URING_ENTRIES, &ctx->ring, 0); if (ret < 0) { - hloge("io_uring_queue_init failed: %d", -ret); + hloge("io_uring_queue_init failed: %d", ret); HV_FREE(ctx); return ret; } From f18010db0244ee7a40f00b521868b3e39d804200 Mon Sep 17 00:00:00 2001 From: ithewei Date: Fri, 22 May 2026 23:12:58 +0800 Subject: [PATCH 6/9] fix: apply suggestions from ai code review --- base/hbase.c | 19 ++++++++++++------- base/hlog.c | 49 ++++++++++++++++++++++++++++++++++++------------- 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/base/hbase.c b/base/hbase.c index 4837aa3ad..69b1d1e1e 100644 --- a/base/hbase.c +++ b/base/hbase.c @@ -450,6 +450,7 @@ time_t hv_parse_time(const char* str) { } int hv_parse_url(hurl_t* stURL, const char* strURL) { + int ret = 0; if (stURL == NULL || strURL == NULL) return -1; memset(stURL, 0, sizeof(hurl_t)); const char* begin = strURL; @@ -504,14 +505,18 @@ int hv_parse_url(hurl_t* stURL, const char* strURL) { unsigned int parsed_port = 0; for (unsigned short i = 1; i <= stURL->fields[HV_URL_PORT].len; ++i) { if (port[i] < '0' || port[i] > '9') { - return -2; + ret = -2; + break; } parsed_port = parsed_port * 10 + (port[i] - '0'); if (parsed_port > 65535) { - return -3; + ret = -3; + break; } } - stURL->port = (unsigned short)parsed_port; + if (ret == 0) { + stURL->port = (unsigned short)parsed_port; + } } else { port = ep; // set default port @@ -526,25 +531,25 @@ int hv_parse_url(hurl_t* stURL, const char* strURL) { stURL->fields[HV_URL_HOST].off = host - begin; stURL->fields[HV_URL_HOST].len = port - host; } - if (ep == end) return 0; + if (ep == end) return ret; // /path sp = ep; ep = strchr(sp, '?'); if (ep == NULL) ep = end; stURL->fields[HV_URL_PATH].off = sp - begin; stURL->fields[HV_URL_PATH].len = ep - sp; - if (ep == end) return 0; + if (ep == end) return ret; // ?query sp = ep + 1; ep = strchr(sp, '#'); if (ep == NULL) ep = end; stURL->fields[HV_URL_QUERY].off = sp - begin; stURL->fields[HV_URL_QUERY].len = ep - sp; - if (ep == end) return 0; + if (ep == end) return ret; // #fragment sp = ep + 1; ep = end; stURL->fields[HV_URL_FRAGMENT].off = sp - begin; stURL->fields[HV_URL_FRAGMENT].len = ep - sp; - return 0; + return ret; } diff --git a/base/hlog.c b/base/hlog.c index ad426ea51..092fe6ae0 100644 --- a/base/hlog.c +++ b/base/hlog.c @@ -29,20 +29,41 @@ //#include "htime.h" #define SECONDS_PER_HOUR 3600 #define SECONDS_PER_DAY 86400 // 24*3600 -#define SECONDS_PER_WEEK 604800 // 7*24*3600; +#define SECONDS_PER_WEEK 604800 // 7*24*3600 + +static inline struct tm* hv_localtime_r(time_t ts, struct tm* tm) { +#ifdef _WIN32 + localtime_s(tm, &ts); +#else + tm = localtime_r(&ts, tm); +#endif + return tm; +} + +static inline struct tm* hv_gmtime_r(time_t ts, struct tm* tm) { +#ifdef _WIN32 + gmtime_s(tm, &ts); +#else + tm = gmtime_r(&ts, tm); +#endif + return tm; +} static int s_gmtoff = 28800; // 8*3600 static void init_gmtoff() { time_t ts = time(NULL); - struct tm* local_tm = localtime(&ts); - struct tm* gmt_tm = gmtime(&ts); - s_gmtoff = (local_tm->tm_hour - gmt_tm->tm_hour) * 3600 + - (local_tm->tm_min - gmt_tm->tm_min) * 60 + - (local_tm->tm_sec - gmt_tm->tm_sec); - - if (local_tm->tm_yday > gmt_tm->tm_yday) { + struct tm local_tm, gmt_tm; + memset(&local_tm, 0, sizeof(local_tm)); + memset(&gmt_tm, 0, sizeof(gmt_tm)); + hv_localtime_r(ts, &local_tm); + hv_gmtime_r(ts, &gmt_tm); + s_gmtoff = (local_tm.tm_hour - gmt_tm.tm_hour) * 3600 + + (local_tm.tm_min - gmt_tm.tm_min) * 60 + + (local_tm.tm_sec - gmt_tm.tm_sec); + + if (local_tm.tm_yday > gmt_tm.tm_yday) { s_gmtoff += SECONDS_PER_DAY; - } else if (local_tm->tm_yday < gmt_tm->tm_yday) { + } else if (local_tm.tm_yday < gmt_tm.tm_yday) { s_gmtoff -= SECONDS_PER_DAY; } } @@ -222,12 +243,14 @@ const char* logger_get_cur_file(logger_t* logger) { } static void logfile_name(const char* filepath, time_t ts, char* buf, int len) { - struct tm* tm = localtime(&ts); + struct tm tm; + memset(&tm, 0, sizeof(tm)); + hv_localtime_r(ts, &tm); snprintf(buf, len, "%s.%04d%02d%02d.log", filepath, - tm->tm_year+1900, - tm->tm_mon+1, - tm->tm_mday); + tm.tm_year+1900, + tm.tm_mon+1, + tm.tm_mday); } static void logfile_truncate(logger_t* logger) { From 9536fa16ef327ae0bd4130865a00f89497e478bd Mon Sep 17 00:00:00 2001 From: ithewei Date: Fri, 22 May 2026 23:16:17 +0800 Subject: [PATCH 7/9] fix: apply suggestions from ai code review --- event/evport.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/event/evport.c b/event/evport.c index 215004988..7ddff7510 100644 --- a/event/evport.c +++ b/event/evport.c @@ -124,6 +124,10 @@ int iowatcher_poll_events(hloop_t* loop, int timeout) { } unsigned nevents = 1; if (port_getn(evport_ctx->port, evport_ctx->events, evport_ctx->capacity, &nevents, tp) != 0) { + if (errno == EINTR) { + return 0; + } + perror("port_getn"); return -1; } for (int i = 0; i < nevents; ++i) { From 4949bf6d41d51881eba276cabbb9e34eca533845 Mon Sep 17 00:00:00 2001 From: ithewei Date: Sat, 23 May 2026 14:19:01 +0800 Subject: [PATCH 8/9] fix: apply suggestions for evpp dir from ai code review --- evpp/Channel.h | 2 ++ evpp/EventLoop.h | 3 +++ evpp/EventLoopThread.h | 1 + evpp/TcpClient.h | 27 ++++++++++++++++++++++++--- evpp/TcpClient_test.cpp | 28 ++++++++++++++-------------- evpp/TcpServer.h | 15 ++++++++++++--- evpp/UdpClient.h | 4 ++++ evpp/UdpClient_test.cpp | 20 ++++++++++---------- evpp/UdpServer.h | 4 ++++ evpp/UdpServer_test.cpp | 16 ++++++++-------- 10 files changed, 82 insertions(+), 38 deletions(-) diff --git a/evpp/Channel.h b/evpp/Channel.h index 17eb58b43..55e078bfb 100644 --- a/evpp/Channel.h +++ b/evpp/Channel.h @@ -13,6 +13,8 @@ namespace hv { +// Channel is a loop-bound wrapper around hio_t. +// The Channel address is stored in hio_context(io), so the object lifetime must cover all pending hio callbacks. class Channel { public: Channel(hio_t* io = NULL) { diff --git a/evpp/EventLoop.h b/evpp/EventLoop.h index e21ee1d33..5e7da9552 100644 --- a/evpp/EventLoop.h +++ b/evpp/EventLoop.h @@ -15,6 +15,8 @@ namespace hv { +// EventLoop is a loop-bound wrapper around hloop_t. +// When constructed with an external hloop_t, the caller remains responsible for that loop's lifetime. class EventLoop : public Status { public: @@ -104,6 +106,7 @@ class EventLoop : public Status { // setTimerInLoop thread-safe TimerID setTimerInLoop(int timeout_ms, TimerCallback cb, uint32_t repeat = INFINITE, TimerID timerID = INVALID_TIMER_ID) { + if (loop_ == NULL) return INVALID_TIMER_ID; if (timerID == INVALID_TIMER_ID) { timerID = generateTimerID(); } diff --git a/evpp/EventLoopThread.h b/evpp/EventLoopThread.h index 51b87b9cc..c876accaa 100644 --- a/evpp/EventLoopThread.h +++ b/evpp/EventLoopThread.h @@ -9,6 +9,7 @@ namespace hv { +// EventLoopThread owns a background thread running one EventLoop. class EventLoopThread : public Status { public: // Return 0 means OK, other failed. diff --git a/evpp/TcpClient.h b/evpp/TcpClient.h index aa3886526..fb6391211 100644 --- a/evpp/TcpClient.h +++ b/evpp/TcpClient.h @@ -11,6 +11,9 @@ namespace hv { template +// TcpClientEventLoopTmpl is a loop-bound wrapper around one outbound connection. +// When bound to an external EventLoopPtr, the caller must ensure the object is stopped and destroyed on the owner loop. +// For long-lived async usage, prefer heap allocation and use stop()/closesocket()/deleteInLoop() as the controlled teardown path. class TcpClientEventLoopTmpl { public: typedef std::shared_ptr TSocketChannelPtr; @@ -23,9 +26,11 @@ class TcpClientEventLoopTmpl { tls_setting = NULL; reconn_setting = NULL; unpack_setting = NULL; + reconn_timer_id = INVALID_TIMER_ID; } virtual ~TcpClientEventLoopTmpl() { + cancelReconnectTimer(); HV_FREE(tls_setting); HV_FREE(reconn_setting); HV_FREE(unpack_setting); @@ -36,6 +41,7 @@ class TcpClientEventLoopTmpl { } // delete thread-safe + // NOTE: This is intended for heap objects that need to be destroyed on the owner loop. void deleteInLoop() { loop_->runInLoop([this](){ delete this; @@ -104,6 +110,7 @@ class TcpClientEventLoopTmpl { } int startConnect() { + loop_->assertInLoopThread(); if (channel == NULL || channel->isClosed()) { int connfd = -1; if (reconn_setting && reconn_setting->cur_retry_cnt > 1) { @@ -172,12 +179,15 @@ class TcpClientEventLoopTmpl { } int startReconnect() { + loop_->assertInLoopThread(); if (!reconn_setting) return -1; if (!reconn_setting_can_retry(reconn_setting)) return -2; uint32_t delay = reconn_setting_calc_delay(reconn_setting); hlogi("reconnect... cnt=%d, delay=%d", reconn_setting->cur_retry_cnt, reconn_setting->cur_delay); - loop_->setTimeout(delay, [this](TimerID timerID){ - (void)(timerID); + reconn_timer_id = loop_->setTimeout(delay, [this](TimerID timerID){ + if (reconn_timer_id == timerID) { + reconn_timer_id = INVALID_TIMER_ID; + } startConnect(); }); return 0; @@ -223,6 +233,7 @@ class TcpClientEventLoopTmpl { void setReconnect(reconn_setting_t* setting) { if (setting == NULL) { + cancelReconnectTimer(); HV_FREE(reconn_setting); return; } @@ -265,7 +276,16 @@ class TcpClientEventLoopTmpl { std::function onWriteComplete; private: - EventLoopPtr loop_; + void cancelReconnectTimer() { + if (reconn_timer_id != INVALID_TIMER_ID) { + loop_->killTimer(reconn_timer_id); + reconn_timer_id = INVALID_TIMER_ID; + } + } + +private: + EventLoopPtr loop_; + TimerID reconn_timer_id; }; template @@ -297,6 +317,7 @@ class TcpClientTmpl : private EventLoopThread, public TcpClientEventLoopTmpl::closesocket(); if (is_loop_owner) { diff --git a/evpp/TcpClient_test.cpp b/evpp/TcpClient_test.cpp index 2ac096e13..c9ec30642 100644 --- a/evpp/TcpClient_test.cpp +++ b/evpp/TcpClient_test.cpp @@ -28,13 +28,13 @@ int main(int argc, char* argv[]) { remote_host = argv[2]; } - TcpClient cli; - int connfd = cli.createsocket(remote_port, remote_host); + auto cli = std::make_shared(); + int connfd = cli->createsocket(remote_port, remote_host); if (connfd < 0) { return -20; } printf("client connect to port %d, connfd=%d ...\n", remote_port, connfd); - cli.onConnection = [&cli](const SocketChannelPtr& channel) { + cli->onConnection = [cli](const SocketChannelPtr& channel) { std::string peeraddr = channel->peeraddr(); if (channel->isConnected()) { printf("connected to %s! connfd=%d\n", peeraddr.c_str(), channel->fd()); @@ -54,11 +54,11 @@ int main(int argc, char* argv[]) { } else { printf("disconnected to %s! connfd=%d\n", peeraddr.c_str(), channel->fd()); } - if (cli.isReconnect()) { - printf("reconnect cnt=%d, delay=%d\n", cli.reconn_setting->cur_retry_cnt, cli.reconn_setting->cur_delay); + if (cli->isReconnect()) { + printf("reconnect cnt=%d, delay=%d\n", cli->reconn_setting->cur_retry_cnt, cli->reconn_setting->cur_delay); } }; - cli.onMessage = [](const SocketChannelPtr& channel, Buffer* buf) { + cli->onMessage = [](const SocketChannelPtr& channel, Buffer* buf) { printf("< %.*s\n", (int)buf->size(), (char*)buf->data()); }; @@ -69,27 +69,27 @@ int main(int argc, char* argv[]) { reconn.min_delay = 1000; reconn.max_delay = 10000; reconn.delay_policy = 2; - cli.setReconnect(&reconn); + cli->setReconnect(&reconn); #endif #if TEST_TLS - cli.withTLS(); + cli->withTLS(); #endif - cli.start(); + cli->start(); std::string str; while (std::getline(std::cin, str)) { if (str == "close") { - cli.closesocket(); + cli->closesocket(); } else if (str == "start") { - cli.start(); + cli->start(); } else if (str == "stop") { - cli.stop(); + cli->stop(true); break; } else { - if (!cli.isConnected()) break; - cli.send(str); + if (!cli->isConnected()) break; + cli->send(str); } } diff --git a/evpp/TcpServer.h b/evpp/TcpServer.h index df6a932c6..251660341 100644 --- a/evpp/TcpServer.h +++ b/evpp/TcpServer.h @@ -11,6 +11,8 @@ namespace hv { template +// TcpServerEventLoopTmpl is a loop-bound wrapper around one listening socket and its accepted channels. +// When an external EventLoopPtr is supplied, the caller remains responsible for owner-loop shutdown and destruction ordering. class TcpServerEventLoopTmpl { public: typedef std::shared_ptr TSocketChannelPtr; @@ -74,6 +76,7 @@ class TcpServerEventLoopTmpl { } int startAccept() { + acceptor_loop->assertInLoopThread(); if (listenfd < 0) { listenfd = createsocket(port, host.c_str()); if (listenfd < 0) { @@ -101,6 +104,7 @@ class TcpServerEventLoopTmpl { } int stopAccept() { + acceptor_loop->assertInLoopThread(); if (listenfd < 0) return -1; hloop_t* loop = acceptor_loop->loop(); if (loop == NULL) return -2; @@ -117,6 +121,7 @@ class TcpServerEventLoopTmpl { acceptor_loop->runInLoop(std::bind(&TcpServerEventLoopTmpl::startAccept, this)); } // stop thread-safe + // NOTE: When an external loop is supplied, this closes the listener but does not own that loop's lifetime. void stop(bool wait_threads_stopped = true) { closesocket(); if (worker_threads.threadNum() > 0) { @@ -173,6 +178,7 @@ class TcpServerEventLoopTmpl { return channels.size(); } + // NOTE: fn is executed while holding mutex_, so it must stay short and must not call server APIs that may lock channels again. int foreachChannel(std::function fn) { std::lock_guard locker(mutex_); for (auto& pair : channels) { @@ -194,16 +200,19 @@ class TcpServerEventLoopTmpl { private: static void newConnEvent(hio_t* connio) { + assert(connio != NULL); TcpServerEventLoopTmpl* server = (TcpServerEventLoopTmpl*)hevent_userdata(connio); + assert(server != NULL); + EventLoop* worker_loop = currentThreadEventLoop; + assert(worker_loop != NULL); if (server->connectionNum() >= server->max_connections) { + --worker_loop->connectionNum; hlogw("over max_connections"); hio_close(connio); return; } // NOTE: attach to worker loop - EventLoop* worker_loop = currentThreadEventLoop; - assert(worker_loop != NULL); hio_attach(worker_loop->loop(), connio); const TSocketChannelPtr& channel = server->addChannel(connio); @@ -229,7 +238,7 @@ class TcpServerEventLoopTmpl { server->onConnection(channel); } server->removeChannel(channel); - // NOTE: After removeChannel, channel may be destroyed, + // NOTE: After removeChannel, channel may be destroyed immediately, // so in this lambda function, no code should be added below. }; diff --git a/evpp/UdpClient.h b/evpp/UdpClient.h index e7bbb6e1c..c85c879cf 100644 --- a/evpp/UdpClient.h +++ b/evpp/UdpClient.h @@ -9,6 +9,8 @@ namespace hv { template +// UdpClientEventLoopTmpl is a loop-bound wrapper around one udp client socket. +// When used with an external EventLoopPtr, the caller must stop receiving and destroy the object on the owner loop. class UdpClientEventLoopTmpl { public: typedef std::shared_ptr TSocketChannelPtr; @@ -72,6 +74,7 @@ class UdpClientEventLoopTmpl { } int startRecv() { + loop_->assertInLoopThread(); if (channel == NULL || channel->isClosed()) { int sockfd = createsocket(remote_port, remote_host.c_str()); if (sockfd < 0) { @@ -179,6 +182,7 @@ class UdpClientTmpl : private EventLoopThread, public UdpClientEventLoopTmpl::closesocket(); if (is_loop_owner) { diff --git a/evpp/UdpClient_test.cpp b/evpp/UdpClient_test.cpp index 4ab91d711..a1775a128 100644 --- a/evpp/UdpClient_test.cpp +++ b/evpp/UdpClient_test.cpp @@ -25,36 +25,36 @@ int main(int argc, char* argv[]) { remote_host = argv[2]; } - UdpClient cli; - int sockfd = cli.createsocket(remote_port, remote_host); + auto cli = std::make_shared(); + int sockfd = cli->createsocket(remote_port, remote_host); if (sockfd < 0) { return -20; } printf("client sendto port %d, sockfd=%d ...\n", remote_port, sockfd); - cli.onMessage = [](const SocketChannelPtr& channel, Buffer* buf) { + cli->onMessage = [](const SocketChannelPtr& channel, Buffer* buf) { printf("< %.*s\n", (int)buf->size(), (char*)buf->data()); }; - cli.start(); + cli->start(); // sendto(time) every 3s - cli.loop()->setInterval(3000, [&cli](TimerID timerID) { + cli->loop()->setInterval(3000, [cli](TimerID timerID) { char str[DATETIME_FMT_BUFLEN] = {0}; datetime_t dt = datetime_now(); datetime_fmt(&dt, str); - cli.sendto(str); + cli->sendto(str); }); std::string str; while (std::getline(std::cin, str)) { if (str == "close") { - cli.closesocket(); + cli->closesocket(); } else if (str == "start") { - cli.start(); + cli->start(); } else if (str == "stop") { - cli.stop(); + cli->stop(true); break; } else { - cli.sendto(str); + cli->sendto(str); } } diff --git a/evpp/UdpServer.h b/evpp/UdpServer.h index 798c8200d..f23d4a07f 100644 --- a/evpp/UdpServer.h +++ b/evpp/UdpServer.h @@ -9,6 +9,8 @@ namespace hv { template +// UdpServerEventLoopTmpl is a loop-bound wrapper around one udp server socket. +// When used with an external EventLoopPtr, stop receiving first and destroy the object on the owner loop. class UdpServerEventLoopTmpl { public: typedef std::shared_ptr TSocketChannelPtr; @@ -48,6 +50,7 @@ class UdpServerEventLoopTmpl { } int startRecv() { + loop_->assertInLoopThread(); if (channel == NULL || channel->isClosed()) { int bindfd = createsocket(port, host.c_str()); if (bindfd < 0) { @@ -153,6 +156,7 @@ class UdpServerTmpl : private EventLoopThread, public UdpServerEventLoopTmpl::closesocket(); if (is_loop_owner) { diff --git a/evpp/UdpServer_test.cpp b/evpp/UdpServer_test.cpp index 7e4fe4677..b7b079a15 100644 --- a/evpp/UdpServer_test.cpp +++ b/evpp/UdpServer_test.cpp @@ -20,30 +20,30 @@ int main(int argc, char* argv[]) { } int port = atoi(argv[1]); - UdpServer srv; - int bindfd = srv.createsocket(port); + auto srv = std::make_shared(); + int bindfd = srv->createsocket(port); if (bindfd < 0) { return -20; } printf("server bind on port %d, bindfd=%d ...\n", port, bindfd); - srv.onMessage = [](const SocketChannelPtr& channel, Buffer* buf) { + srv->onMessage = [](const SocketChannelPtr& channel, Buffer* buf) { // echo printf("< %.*s\n", (int)buf->size(), (char*)buf->data()); channel->write(buf); }; - srv.start(); + srv->start(); std::string str; while (std::getline(std::cin, str)) { if (str == "close") { - srv.closesocket(); + srv->closesocket(); } else if (str == "start") { - srv.start(); + srv->start(); } else if (str == "stop") { - srv.stop(); + srv->stop(true); break; } else { - srv.sendto(str); + srv->sendto(str); } } From 475a5959d2621905f5d9f79a39931f727c070cf0 Mon Sep 17 00:00:00 2001 From: ithewei Date: Sat, 23 May 2026 23:36:57 +0800 Subject: [PATCH 9/9] fix: apply suggestions for http dir from ai code review --- http/HttpMessage.h | 4 +++- http/server/HttpHandler.cpp | 15 +++++++++------ http/server/HttpMiddleware.cpp | 6 +++++- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/http/HttpMessage.h b/http/HttpMessage.h index 55e5c62be..5be241957 100644 --- a/http/HttpMessage.h +++ b/http/HttpMessage.h @@ -219,7 +219,9 @@ class HV_EXPORT HttpMessage { if (file.open(filepath.c_str(), "wb") != 0) { return HTTP_STATUS_INTERNAL_SERVER_ERROR; } - file.write(formdata.content.data(), formdata.content.size()); + if (file.write(formdata.content.data(), formdata.content.size()) != formdata.content.size()) { + return HTTP_STATUS_INTERNAL_SERVER_ERROR; + } return 200; } diff --git a/http/server/HttpHandler.cpp b/http/server/HttpHandler.cpp index 7cc21a99d..477a8469c 100644 --- a/http/server/HttpHandler.cpp +++ b/http/server/HttpHandler.cpp @@ -586,6 +586,11 @@ int HttpHandler::defaultStaticHandler() { } long total = file->size(); if (to == 0 || to >= total) to = total - 1; + if (from < 0 || from >= total || to < from) { + closeFile(); + resp->SetHeader("Content-Range", hv::asprintf("bytes */%ld", total)); + return HTTP_STATUS_RANGE_NOT_SATISFIABLE; + } file->seek(from); status_code = HTTP_STATUS_PARTIAL_CONTENT; resp->status_code = HTTP_STATUS_PARTIAL_CONTENT; @@ -1046,9 +1051,8 @@ int HttpHandler::handleForwardProxy() { return connectProxy(req->url); } else { hlogw("[%s:%d] Forbidden to forward proxy %s", ip, port, req->url.c_str()); - SetError(HTTP_STATUS_FORBIDDEN, HTTP_STATUS_FORBIDDEN); + return SendHttpStatusResponse(HTTP_STATUS_FORBIDDEN); } - return 0; } int HttpHandler::handleReverseProxy() { @@ -1079,8 +1083,7 @@ int HttpHandler::connectProxy(const std::string& strUrl) { if (forward_proxy && !service->IsTrustProxy(url.host.c_str())) { hlogw("[%s:%d] Forbidden to proxy %s", ip, port, url.host.c_str()); - SetError(HTTP_STATUS_FORBIDDEN, HTTP_STATUS_FORBIDDEN); - return 0; + return SendHttpStatusResponse(HTTP_STATUS_FORBIDDEN); } hloop_t* loop = hevent_loop(io); @@ -1102,10 +1105,10 @@ int HttpHandler::connectProxy(const std::string& strUrl) { hio_set_connect_timeout(upstream_io, service->proxy_connect_timeout); } if (service->proxy_read_timeout > 0) { - hio_set_read_timeout(io, service->proxy_read_timeout); + hio_set_read_timeout(upstream_io, service->proxy_read_timeout); } if (service->proxy_write_timeout > 0) { - hio_set_write_timeout(io, service->proxy_write_timeout); + hio_set_write_timeout(upstream_io, service->proxy_write_timeout); } hio_connect(upstream_io); // NOTE: wait upstream_io connected then start read diff --git a/http/server/HttpMiddleware.cpp b/http/server/HttpMiddleware.cpp index 45607908c..c2a4f7bdd 100644 --- a/http/server/HttpMiddleware.cpp +++ b/http/server/HttpMiddleware.cpp @@ -4,7 +4,11 @@ BEGIN_NAMESPACE_HV int HttpMiddleware::CORS(HttpRequest* req, HttpResponse* resp) { - resp->headers["Access-Control-Allow-Origin"] = req->GetHeader("Origin", "*"); + std::string origin = req->GetHeader("Origin", "*"); + resp->headers["Access-Control-Allow-Origin"] = origin; + if (origin != "*") { + resp->headers["Vary"] = "Origin"; + } if (req->method == HTTP_OPTIONS) { resp->headers["Access-Control-Allow-Methods"] = req->GetHeader("Access-Control-Request-Method", "OPTIONS, HEAD, GET, POST, PUT, DELETE, PATCH"); resp->headers["Access-Control-Allow-Headers"] = req->GetHeader("Access-Control-Request-Headers", "Content-Type");