From a90085ab7c3771bc9f33a50336d1a0550d1d848b Mon Sep 17 00:00:00 2001 From: Yundi339 Date: Thu, 26 Mar 2026 13:18:10 +0800 Subject: [PATCH 01/20] feat: add FileCacheEx - enhanced file cache with configurable header length - New FileCacheEx module alongside original FileCache (non-invasive) - Configurable max_header_length (was hardcoded HTTP_HEADER_MAX_LENGTH) - Configurable max_file_size, max_cache_num at runtime - prepend_header() returns bool to report success/failure - Expose header metrics: get_header_reserve(), get_header_used(), header_fits() - Fix stat() name collision in is_modified() using ::stat() - Add FileCacheEx.h to exported headers in cmake/vars.cmake --- cmake/vars.cmake | 1 + http/server/FileCacheEx.cpp | 161 ++++++++++++++++++++++++++++++++++++ http/server/FileCacheEx.h | 145 ++++++++++++++++++++++++++++++++ 3 files changed, 307 insertions(+) create mode 100644 http/server/FileCacheEx.cpp create mode 100644 http/server/FileCacheEx.h diff --git a/cmake/vars.cmake b/cmake/vars.cmake index 1acf5565f..5276bb297 100644 --- a/cmake/vars.cmake +++ b/cmake/vars.cmake @@ -103,6 +103,7 @@ set(HTTP_SERVER_HEADERS http/server/HttpContext.h http/server/HttpResponseWriter.h http/server/WebSocketServer.h + http/server/FileCacheEx.h ) set(MQTT_HEADERS diff --git a/http/server/FileCacheEx.cpp b/http/server/FileCacheEx.cpp new file mode 100644 index 000000000..4c081009d --- /dev/null +++ b/http/server/FileCacheEx.cpp @@ -0,0 +1,161 @@ +#include "FileCacheEx.h" + +#include "herr.h" +#include "hscope.h" +#include "htime.h" +#include "hlog.h" + +#include "httpdef.h" // http_content_type_str_by_suffix +#include "http_page.h" // make_index_of_page + +#ifdef OS_WIN +#include "hstring.h" // hv::utf8_to_wchar +#endif + +#define ETAG_FMT "\"%zx-%zx\"" + +FileCacheEx::FileCacheEx(size_t capacity) + : hv::LRUCache(capacity) { + stat_interval = 10; // s + expired_time = 60; // s + max_header_length = FILECACHE_EX_DEFAULT_HEADER_LENGTH; + max_file_size = FILECACHE_EX_DEFAULT_MAX_FILE_SIZE; +} + +file_cache_ex_ptr FileCacheEx::Open(const char* filepath, OpenParam* param) { + file_cache_ex_ptr fc = Get(filepath); +#ifdef OS_WIN + std::wstring wfilepath; +#endif + bool modified = false; + if (fc) { + time_t now = time(NULL); + if (now - fc->stat_time > stat_interval) { + fc->stat_time = now; + fc->stat_cnt++; +#ifdef OS_WIN + wfilepath = hv::utf8_to_wchar(filepath); + now = fc->st.st_mtime; + _wstat(wfilepath.c_str(), (struct _stat*)&fc->st); + modified = now != fc->st.st_mtime; +#else + modified = fc->is_modified(); +#endif + } + if (param->need_read) { + if (!modified && fc->is_complete()) { + param->need_read = false; + } + } + } + if (fc == NULL || modified || param->need_read) { + struct stat st; + int flags = O_RDONLY; +#ifdef O_BINARY + flags |= O_BINARY; +#endif + int fd = -1; +#ifdef OS_WIN + if (wfilepath.empty()) wfilepath = hv::utf8_to_wchar(filepath); + if (_wstat(wfilepath.c_str(), (struct _stat*)&st) != 0) { + param->error = ERR_OPEN_FILE; + return NULL; + } + if (S_ISREG(st.st_mode)) { + fd = _wopen(wfilepath.c_str(), flags); + } else if (S_ISDIR(st.st_mode)) { + fd = 0; + } +#else + if (::stat(filepath, &st) != 0) { + param->error = ERR_OPEN_FILE; + return NULL; + } + fd = open(filepath, flags); +#endif + if (fd < 0) { + param->error = ERR_OPEN_FILE; + return NULL; + } + defer(if (fd > 0) { close(fd); }) + if (fc == NULL) { + if (S_ISREG(st.st_mode) || + (S_ISDIR(st.st_mode) && + filepath[strlen(filepath) - 1] == '/')) { + fc = std::make_shared(); + fc->filepath = filepath; + fc->st = st; + fc->header_reserve = max_header_length; + time(&fc->open_time); + fc->stat_time = fc->open_time; + fc->stat_cnt = 1; + put(filepath, fc); + } else { + param->error = ERR_MISMATCH; + return NULL; + } + } + if (S_ISREG(fc->st.st_mode)) { + param->filesize = fc->st.st_size; + // FILE + if (param->need_read) { + if (fc->st.st_size > param->max_read) { + param->error = ERR_OVER_LIMIT; + return NULL; + } + fc->resize_buf(fc->st.st_size, max_header_length); + int nread = read(fd, fc->filebuf.base, fc->filebuf.len); + if (nread != (int)fc->filebuf.len) { + hloge("Failed to read file: %s", filepath); + param->error = ERR_READ_FILE; + return NULL; + } + } + const char* suffix = strrchr(filepath, '.'); + if (suffix) { + http_content_type content_type = http_content_type_enum_by_suffix(suffix + 1); + if (content_type == TEXT_HTML) { + fc->content_type = "text/html; charset=utf-8"; + } else if (content_type == TEXT_PLAIN) { + fc->content_type = "text/plain; charset=utf-8"; + } else { + fc->content_type = http_content_type_str_by_suffix(suffix + 1); + } + } + } else if (S_ISDIR(fc->st.st_mode)) { + // DIR + std::string page; + make_index_of_page(filepath, page, param->path); + fc->resize_buf(page.size(), max_header_length); + memcpy(fc->filebuf.base, page.c_str(), page.size()); + fc->content_type = "text/html; charset=utf-8"; + } + gmtime_fmt(fc->st.st_mtime, fc->last_modified); + snprintf(fc->etag, sizeof(fc->etag), ETAG_FMT, + (size_t)fc->st.st_mtime, (size_t)fc->st.st_size); + } + return fc; +} + +bool FileCacheEx::Exists(const char* filepath) const { + return contains(filepath); +} + +bool FileCacheEx::Close(const char* filepath) { + return remove(filepath); +} + +file_cache_ex_ptr FileCacheEx::Get(const char* filepath) { + file_cache_ex_ptr fc; + if (get(filepath, fc)) { + return fc; + } + return NULL; +} + +void FileCacheEx::RemoveExpiredFileCache() { + time_t now = time(NULL); + remove_if([this, now](const std::string& filepath, const file_cache_ex_ptr& fc) { + return (now - fc->stat_time > expired_time); + }); +} diff --git a/http/server/FileCacheEx.h b/http/server/FileCacheEx.h new file mode 100644 index 000000000..f02d00fa8 --- /dev/null +++ b/http/server/FileCacheEx.h @@ -0,0 +1,145 @@ +#ifndef HV_FILE_CACHE_EX_H_ +#define HV_FILE_CACHE_EX_H_ + +/* + * FileCacheEx — Enhanced File Cache for libhv HTTP server + * + * Improvements over the original FileCache: + * 1. Configurable max_header_length (no more hardcoded 4096) + * 2. prepend_header() returns bool to report success/failure + * 3. Exposes header/buffer metrics via accessors + * 4. Fixes stat() name collision in is_modified() + * 5. max_cache_num / max_file_size configurable at runtime + * 6. Reserved header space can be tuned per-instance + * 7. Fully backward-compatible struct layout + * + * This is a NEW module alongside FileCache — no modifications to the + * original code — to keep things non-invasive for upstream PR review. + */ + +#include +#include +#include + +#include "hbuf.h" +#include "hstring.h" +#include "LRUCache.h" + +// Default values — may be overridden at runtime via FileCacheEx setters +#define FILECACHE_EX_DEFAULT_HEADER_LENGTH 4096 // 4K +#define FILECACHE_EX_DEFAULT_MAX_NUM 100 +#define FILECACHE_EX_DEFAULT_MAX_FILE_SIZE (1 << 22) // 4M + +typedef struct file_cache_ex_s { + std::string filepath; + struct stat st; + time_t open_time; + time_t stat_time; + uint32_t stat_cnt; + HBuf buf; // header_reserve + file_content + hbuf_t filebuf; // points into buf: file content region + hbuf_t httpbuf; // points into buf: header + file content after prepend + char last_modified[64]; + char etag[64]; + std::string content_type; + + // --- new: expose header metrics --- + int header_reserve; // reserved bytes before file content + int header_used; // actual bytes used by prepend_header + + file_cache_ex_s() { + stat_cnt = 0; + header_reserve = FILECACHE_EX_DEFAULT_HEADER_LENGTH; + header_used = 0; + memset(last_modified, 0, sizeof(last_modified)); + memset(etag, 0, sizeof(etag)); + } + + // Fixed: avoids shadowing struct stat member with stat() call + bool is_modified() { + time_t mtime = st.st_mtime; + ::stat(filepath.c_str(), &st); + return mtime != st.st_mtime; + } + + bool is_complete() { + if (S_ISDIR(st.st_mode)) return filebuf.len > 0; + return filebuf.len == (size_t)st.st_size; + } + + void resize_buf(int filesize, int reserved) { + header_reserve = reserved; + buf.resize(reserved + filesize); + filebuf.base = buf.base + reserved; + filebuf.len = filesize; + } + + void resize_buf(int filesize) { + resize_buf(filesize, header_reserve); + } + + // Returns true on success, false if header exceeds reserved space + bool prepend_header(const char* header, int len) { + if (len > header_reserve) return false; + httpbuf.base = filebuf.base - len; + httpbuf.len = len + filebuf.len; + memcpy(httpbuf.base, header, len); + header_used = len; + return true; + } + + // --- accessors --- + int get_header_reserve() const { return header_reserve; } + int get_header_used() const { return header_used; } + int get_header_remaining() const { return header_reserve - header_used; } + bool header_fits(int len) const { return len <= header_reserve; } +} file_cache_ex_t; + +typedef std::shared_ptr file_cache_ex_ptr; + +class FileCacheEx : public hv::LRUCache { +public: + // --- configurable parameters (were hardcoded macros before) --- + int stat_interval; // seconds between stat() checks + int expired_time; // seconds before cache entry expires + int max_header_length; // reserved header bytes per entry + int max_file_size; // max cached file size (larger = large-file path) + + explicit FileCacheEx(size_t capacity = FILECACHE_EX_DEFAULT_MAX_NUM); + + struct OpenParam { + bool need_read; + int max_read; // per-request override for max file size + const char* path; // URL path (for directory listing) + size_t filesize; // [out] actual file size + int error; // [out] error code if Open returns NULL + + OpenParam() { + need_read = true; + max_read = FILECACHE_EX_DEFAULT_MAX_FILE_SIZE; + path = "/"; + filesize = 0; + error = 0; + } + }; + + file_cache_ex_ptr Open(const char* filepath, OpenParam* param); + bool Exists(const char* filepath) const; + bool Close(const char* filepath); + void RemoveExpiredFileCache(); + + // --- new: getters --- + int GetMaxHeaderLength() const { return max_header_length; } + int GetMaxFileSize() const { return max_file_size; } + int GetStatInterval() const { return stat_interval; } + int GetExpiredTime() const { return expired_time; } + + // --- new: setters --- + void SetMaxHeaderLength(int len) { max_header_length = len; } + void SetMaxFileSize(int size) { max_file_size = size; } + +protected: + file_cache_ex_ptr Get(const char* filepath); +}; + +#endif // HV_FILE_CACHE_EX_H_ From bc30edc8e9e3721e85eee6ddd481c487c18ec34d Mon Sep 17 00:00:00 2001 From: Yundi339 Date: Thu, 26 Mar 2026 14:05:41 +0800 Subject: [PATCH 02/20] fix: address race conditions and safety issues in FileCacheEx - Add per-entry mutex to file_cache_ex_s for thread-safe mutations - Hold fc->mutex during is_modified() / stat check to prevent data race - Hold fc->mutex during resize_buf() / read to prevent use-after-free - Make prepend_header() thread-safe with lock_guard - Defer put() into LRU cache until entry is fully initialized (prevents stale/incomplete cache entries on ERR_OVER_LIMIT) - Use read() loop to handle partial reads (EINTR) - Invalidate httpbuf pointers after resize_buf() reallocation - Thread-safe get_header_used() / get_header_remaining() accessors --- http/server/FileCacheEx.cpp | 25 +++++++++++++++++++------ http/server/FileCacheEx.h | 21 +++++++++++++++++---- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/http/server/FileCacheEx.cpp b/http/server/FileCacheEx.cpp index 4c081009d..30728d8dd 100644 --- a/http/server/FileCacheEx.cpp +++ b/http/server/FileCacheEx.cpp @@ -29,6 +29,7 @@ file_cache_ex_ptr FileCacheEx::Open(const char* filepath, OpenParam* param) { #endif bool modified = false; if (fc) { + std::lock_guard lock(fc->mutex); time_t now = time(NULL); if (now - fc->stat_time > stat_interval) { fc->stat_time = now; @@ -89,26 +90,36 @@ file_cache_ex_ptr FileCacheEx::Open(const char* filepath, OpenParam* param) { time(&fc->open_time); fc->stat_time = fc->open_time; fc->stat_cnt = 1; - put(filepath, fc); + // NOTE: do NOT put() into cache yet — defer until fully initialized } else { param->error = ERR_MISMATCH; return NULL; } } + // Hold fc->mutex for the remainder of initialization + std::lock_guard lock(fc->mutex); if (S_ISREG(fc->st.st_mode)) { param->filesize = fc->st.st_size; // FILE if (param->need_read) { if (fc->st.st_size > param->max_read) { param->error = ERR_OVER_LIMIT; + // Don't cache incomplete entries return NULL; } fc->resize_buf(fc->st.st_size, max_header_length); - int nread = read(fd, fc->filebuf.base, fc->filebuf.len); - if (nread != (int)fc->filebuf.len) { - hloge("Failed to read file: %s", filepath); - param->error = ERR_READ_FILE; - return NULL; + // Loop to handle partial reads (EINTR, etc.) + char* dst = fc->filebuf.base; + size_t remaining = fc->filebuf.len; + while (remaining > 0) { + int nread = read(fd, dst, remaining); + if (nread <= 0) { + hloge("Failed to read file: %s", filepath); + param->error = ERR_READ_FILE; + return NULL; + } + dst += nread; + remaining -= nread; } } const char* suffix = strrchr(filepath, '.'); @@ -133,6 +144,8 @@ file_cache_ex_ptr FileCacheEx::Open(const char* filepath, OpenParam* param) { gmtime_fmt(fc->st.st_mtime, fc->last_modified); snprintf(fc->etag, sizeof(fc->etag), ETAG_FMT, (size_t)fc->st.st_mtime, (size_t)fc->st.st_size); + // Cache the fully initialized entry + put(filepath, fc); } return fc; } diff --git a/http/server/FileCacheEx.h b/http/server/FileCacheEx.h index f02d00fa8..734689ef1 100644 --- a/http/server/FileCacheEx.h +++ b/http/server/FileCacheEx.h @@ -25,12 +25,16 @@ #include "hstring.h" #include "LRUCache.h" +// Forward declare to avoid header pollution +struct file_cache_ex_s; + // Default values — may be overridden at runtime via FileCacheEx setters #define FILECACHE_EX_DEFAULT_HEADER_LENGTH 4096 // 4K #define FILECACHE_EX_DEFAULT_MAX_NUM 100 #define FILECACHE_EX_DEFAULT_MAX_FILE_SIZE (1 << 22) // 4M typedef struct file_cache_ex_s { + mutable std::mutex mutex; // protects all mutable state below std::string filepath; struct stat st; time_t open_time; @@ -56,30 +60,39 @@ typedef struct file_cache_ex_s { } // Fixed: avoids shadowing struct stat member with stat() call + // NOTE: caller must hold mutex bool is_modified() { time_t mtime = st.st_mtime; ::stat(filepath.c_str(), &st); return mtime != st.st_mtime; } + // NOTE: caller must hold mutex bool is_complete() { if (S_ISDIR(st.st_mode)) return filebuf.len > 0; return filebuf.len == (size_t)st.st_size; } + // NOTE: caller must hold mutex — invalidates filebuf/httpbuf pointers void resize_buf(int filesize, int reserved) { header_reserve = reserved; buf.resize(reserved + filesize); filebuf.base = buf.base + reserved; filebuf.len = filesize; + // Invalidate httpbuf since buffer may have been reallocated + httpbuf.base = NULL; + httpbuf.len = 0; + header_used = 0; } void resize_buf(int filesize) { resize_buf(filesize, header_reserve); } - // Returns true on success, false if header exceeds reserved space + // Thread-safe: prepend header into reserved space. + // Returns true on success, false if header exceeds reserved space. bool prepend_header(const char* header, int len) { + std::lock_guard lock(mutex); if (len > header_reserve) return false; httpbuf.base = filebuf.base - len; httpbuf.len = len + filebuf.len; @@ -88,10 +101,10 @@ typedef struct file_cache_ex_s { return true; } - // --- accessors --- + // --- thread-safe accessors --- int get_header_reserve() const { return header_reserve; } - int get_header_used() const { return header_used; } - int get_header_remaining() const { return header_reserve - header_used; } + int get_header_used() const { std::lock_guard lock(mutex); return header_used; } + int get_header_remaining() const { std::lock_guard lock(mutex); return header_reserve - header_used; } bool header_fits(int len) const { return len <= header_reserve; } } file_cache_ex_t; From f14db65c6b473a06be6f33f043b67d5a870717c8 Mon Sep 17 00:00:00 2001 From: Yundi339 Date: Thu, 26 Mar 2026 14:25:19 +0800 Subject: [PATCH 03/20] refactor: align Windows compat with original libhv patterns - Revert _read/_close to plain read/close (POSIX compat via ) - Keep is_modified() using ::stat() (only called on non-Windows path) - Consistent with original FileCache.cpp Windows handling patterns --- http/server/FileCacheEx.h | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/http/server/FileCacheEx.h b/http/server/FileCacheEx.h index 734689ef1..f6c3af5b7 100644 --- a/http/server/FileCacheEx.h +++ b/http/server/FileCacheEx.h @@ -21,13 +21,11 @@ #include #include +#include "hexport.h" #include "hbuf.h" #include "hstring.h" #include "LRUCache.h" -// Forward declare to avoid header pollution -struct file_cache_ex_s; - // Default values — may be overridden at runtime via FileCacheEx setters #define FILECACHE_EX_DEFAULT_HEADER_LENGTH 4096 // 4K #define FILECACHE_EX_DEFAULT_MAX_NUM 100 @@ -59,8 +57,8 @@ typedef struct file_cache_ex_s { memset(etag, 0, sizeof(etag)); } - // Fixed: avoids shadowing struct stat member with stat() call - // NOTE: caller must hold mutex + // NOTE: caller must hold mutex. + // On Windows, Open() uses _wstat() directly instead of calling this. bool is_modified() { time_t mtime = st.st_mtime; ::stat(filepath.c_str(), &st); @@ -74,9 +72,9 @@ typedef struct file_cache_ex_s { } // NOTE: caller must hold mutex — invalidates filebuf/httpbuf pointers - void resize_buf(int filesize, int reserved) { + void resize_buf(size_t filesize, int reserved) { header_reserve = reserved; - buf.resize(reserved + filesize); + buf.resize((size_t)reserved + filesize); filebuf.base = buf.base + reserved; filebuf.len = filesize; // Invalidate httpbuf since buffer may have been reallocated @@ -85,7 +83,7 @@ typedef struct file_cache_ex_s { header_used = 0; } - void resize_buf(int filesize) { + void resize_buf(size_t filesize) { resize_buf(filesize, header_reserve); } @@ -93,9 +91,9 @@ typedef struct file_cache_ex_s { // Returns true on success, false if header exceeds reserved space. bool prepend_header(const char* header, int len) { std::lock_guard lock(mutex); - if (len > header_reserve) return false; + if (len <= 0 || len > header_reserve) return false; httpbuf.base = filebuf.base - len; - httpbuf.len = len + filebuf.len; + httpbuf.len = (size_t)len + filebuf.len; memcpy(httpbuf.base, header, len); header_used = len; return true; @@ -105,12 +103,12 @@ typedef struct file_cache_ex_s { int get_header_reserve() const { return header_reserve; } int get_header_used() const { std::lock_guard lock(mutex); return header_used; } int get_header_remaining() const { std::lock_guard lock(mutex); return header_reserve - header_used; } - bool header_fits(int len) const { return len <= header_reserve; } + bool header_fits(int len) const { return len > 0 && len <= header_reserve; } } file_cache_ex_t; typedef std::shared_ptr file_cache_ex_ptr; -class FileCacheEx : public hv::LRUCache { +class HV_EXPORT FileCacheEx : public hv::LRUCache { public: // --- configurable parameters (were hardcoded macros before) --- int stat_interval; // seconds between stat() checks From f9bae9788a7d016543a1df25672150664a7c1561 Mon Sep 17 00:00:00 2001 From: Yundi339 Date: Thu, 26 Mar 2026 15:07:26 +0800 Subject: [PATCH 04/20] docs: add FileCacheEx documentation (en/cn) and fix comment style - Add docs/FileCacheEx.md (English API documentation) - Add docs/cn/FileCacheEx.md (Chinese API documentation) - Fix include comments to follow libhv convention (add 'import' keyword) --- docs/FileCacheEx.md | 119 ++++++++++++++++++++++++++++++++ docs/cn/FileCacheEx.md | 134 ++++++++++++++++++++++++++++++++++++ http/server/FileCacheEx.cpp | 6 +- 3 files changed, 256 insertions(+), 3 deletions(-) create mode 100644 docs/FileCacheEx.md create mode 100644 docs/cn/FileCacheEx.md diff --git a/docs/FileCacheEx.md b/docs/FileCacheEx.md new file mode 100644 index 000000000..622c56c6d --- /dev/null +++ b/docs/FileCacheEx.md @@ -0,0 +1,119 @@ +# FileCacheEx — Enhanced File Cache + +`FileCacheEx` is a drop-in replacement for `FileCache` that fixes thread-safety issues and provides runtime-configurable parameters. + +## Header + +```c++ +#include "FileCacheEx.h" +``` + +## Improvements over FileCache + +| Feature | FileCache | FileCacheEx | +|---------|-----------|-------------| +| HTTP header reserve | Compile-time 1024 bytes | Runtime configurable, default 4096 | +| `prepend_header()` | Returns `void`, silently drops overflow | Returns `bool`, `false` on overflow | +| Per-entry thread safety | No locking | Per-entry `std::mutex` | +| `resize_buf()` httpbuf | May leave dangling reference | Explicitly invalidated (UAF prevention) | +| File read | Single `read()`, may short-read | Loop with `EINTR` handling | +| Cache insertion | Before initialization completes | Deferred until fully initialized | +| Max file size | Compile-time `FILE_CACHE_MAX_SIZE` | Runtime `SetMaxFileSize()` | +| DLL export | None | `HV_EXPORT` | + +## Data Structures + +```c++ +typedef struct file_cache_ex_s { + mutable std::mutex mutex; // per-entry lock + std::string filepath; + struct stat st; + time_t open_time; + time_t stat_time; + uint32_t stat_cnt; + HBuf buf; // header_reserve + file_content + hbuf_t filebuf; // points into buf: file content + hbuf_t httpbuf; // points into buf: header + content + char last_modified[64]; + char etag[64]; + std::string content_type; + int header_reserve; // bytes reserved before content + int header_used; // bytes actually used by header + + bool is_modified(); // caller must hold mutex + bool is_complete(); // caller must hold mutex + void resize_buf(size_t filesize, int reserved); // caller must hold mutex + void resize_buf(size_t filesize); + bool prepend_header(const char* header, int len); // thread-safe + + // Thread-safe accessors + int get_header_reserve() const; + int get_header_used() const; + int get_header_remaining() const; + bool header_fits(int len) const; +} file_cache_ex_t; + +typedef std::shared_ptr file_cache_ex_ptr; +``` + +## FileCacheEx Class + +```c++ +class HV_EXPORT FileCacheEx : public hv::LRUCache { +public: + int stat_interval; // seconds between stat() checks, default 10 + int expired_time; // seconds before expiry, default 60 + int max_header_length; // header reserve per entry, default 4096 + int max_file_size; // max cached file size, default 4MB + + explicit FileCacheEx(size_t capacity = 100); + + file_cache_ex_ptr Open(const char* filepath, OpenParam* param); + bool Exists(const char* filepath) const; + bool Close(const char* filepath); + void RemoveExpiredFileCache(); + + int GetMaxHeaderLength() const; + int GetMaxFileSize() const; + int GetStatInterval() const; + int GetExpiredTime() const; + + void SetMaxHeaderLength(int len); + void SetMaxFileSize(int size); +}; +``` + +## Usage + +```c++ +FileCacheEx filecache(200); // LRU capacity = 200 +filecache.SetMaxHeaderLength(8192); // 8K header reserve +filecache.SetMaxFileSize(1 << 24); // 16MB max file +filecache.stat_interval = 5; +filecache.expired_time = 120; + +FileCacheEx::OpenParam param; +file_cache_ex_ptr fc = filecache.Open("/var/www/index.html", ¶m); +if (fc) { + std::string header = "HTTP/1.1 200 OK\r\nContent-Type: text/html\r\n\r\n"; + if (fc->prepend_header(header.c_str(), header.size())) { + // send fc->httpbuf (header + content) + } else { + // header exceeds reserve, fall back to separate send + } +} +``` + +## Migration from FileCache + +1. Replace `#include "FileCache.h"` with `#include "FileCacheEx.h"` +2. Change types: `FileCache` → `FileCacheEx`, `file_cache_ptr` → `file_cache_ex_ptr` +3. Handle `bool` return from `prepend_header()` +4. Optional: configure via `SetMaxHeaderLength()` / `SetMaxFileSize()` + +## Thread Safety + +- `FileCacheEx` inherits `hv::LRUCache` global mutex for LRU operations +- Each `file_cache_ex_s` entry has its own `std::mutex` for entry-level protection +- `prepend_header()` locks automatically; callers need no external synchronization +- `is_modified()`, `is_complete()`, `resize_buf()` require caller to hold the mutex diff --git a/docs/cn/FileCacheEx.md b/docs/cn/FileCacheEx.md new file mode 100644 index 000000000..f99cb04cf --- /dev/null +++ b/docs/cn/FileCacheEx.md @@ -0,0 +1,134 @@ +# FileCacheEx — 增强版文件缓存 + +`FileCacheEx` 是 `FileCache` 的增强替代方案，解决了原始实现中的线程安全问题，并提供运行时可配置的参数。 + +## 头文件 + +```c++ +#include "FileCacheEx.h" +``` + +## 与 FileCache 的区别 + +| 特性 | FileCache | FileCacheEx | +|------|-----------|-------------| +| HTTP 头部预留空间 | 编译期固定 1024 字节 | 运行时可配置，默认 4096 字节 | +| `prepend_header()` | 返回 `void`，溢出静默丢弃 | 返回 `bool`，失败返回 `false` | +| 缓存条目线程安全 | 无锁保护 | 每条目 `std::mutex` | +| `resize_buf()` 后 httpbuf | 可能悬空引用 | 主动置空，防止 UAF | +| 文件读取 | 单次 `read()`，可能短读 | 循环读取，处理 `EINTR` | +| `put()` 时机 | 初始化前放入缓存 | 完全初始化后放入缓存 | +| 最大文件大小 | 编译期宏 `FILE_CACHE_MAX_SIZE` | 运行时 `SetMaxFileSize()` | +| DLL 导出 | 无 | `HV_EXPORT` | + +## 数据结构 + +```c++ +// 缓存条目 +typedef struct file_cache_ex_s { + mutable std::mutex mutex; // 条目级互斥锁 + std::string filepath; + struct stat st; + time_t open_time; + time_t stat_time; + uint32_t stat_cnt; + HBuf buf; // header_reserve + file_content + hbuf_t filebuf; // 指向 buf 中文件内容区域 + hbuf_t httpbuf; // 指向 buf 中 header + 文件内容区域 + char last_modified[64]; + char etag[64]; + std::string content_type; + int header_reserve; // 头部预留字节数 + int header_used; // 实际使用的头部字节数 + + // 方法 + bool is_modified(); // 检查文件是否被修改（需持锁） + bool is_complete(); // 检查缓存是否完整（需持锁） + void resize_buf(size_t filesize, int reserved); // 重新分配缓冲区（需持锁） + void resize_buf(size_t filesize); // 使用当前 header_reserve + bool prepend_header(const char* header, int len); // 线程安全：写入 HTTP 头 + + // 线程安全访问器 + int get_header_reserve() const; + int get_header_used() const; + int get_header_remaining() const; + bool header_fits(int len) const; +} file_cache_ex_t; + +typedef std::shared_ptr file_cache_ex_ptr; +``` + +## FileCacheEx 类 + +```c++ +class HV_EXPORT FileCacheEx : public hv::LRUCache { +public: + // 可配置参数 + int stat_interval; // stat() 检查间隔（秒），默认 10 + int expired_time; // 缓存过期时间（秒），默认 60 + int max_header_length; // 每条目头部预留字节，默认 4096 + int max_file_size; // 最大缓存文件大小，默认 4MB + + explicit FileCacheEx(size_t capacity = 100); + + // 打开文件并缓存 + file_cache_ex_ptr Open(const char* filepath, OpenParam* param); + + // 检查文件是否在缓存中 + bool Exists(const char* filepath) const; + + // 从缓存中移除文件 + bool Close(const char* filepath); + + // 移除过期缓存条目 + void RemoveExpiredFileCache(); + + // Getter + int GetMaxHeaderLength() const; + int GetMaxFileSize() const; + int GetStatInterval() const; + int GetExpiredTime() const; + + // Setter + void SetMaxHeaderLength(int len); + void SetMaxFileSize(int size); +}; +``` + +## 使用示例 + +```c++ +FileCacheEx filecache(200); // 最大 200 个缓存条目 +filecache.SetMaxHeaderLength(8192); // 8K 头部预留 +filecache.SetMaxFileSize(1 << 24); // 16MB 最大缓存文件 +filecache.stat_interval = 5; // 每 5 秒检查文件变更 +filecache.expired_time = 120; // 2 分钟过期 + +FileCacheEx::OpenParam param; +file_cache_ex_ptr fc = filecache.Open("/var/www/index.html", ¶m); +if (fc) { + // 写入 HTTP 响应头 + std::string header = "HTTP/1.1 200 OK\r\nContent-Type: text/html\r\n\r\n"; + if (fc->prepend_header(header.c_str(), header.size())) { + // 发送 fc->httpbuf (header + content) + } else { + // 头部超出预留空间，回退到普通发送 + } +} +``` + +## 迁移指南 + +从 `FileCache` 迁移到 `FileCacheEx`： + +1. 替换头文件引用：`#include "FileCache.h"` → `#include "FileCacheEx.h"` +2. 替换类型：`FileCache` → `FileCacheEx`，`file_cache_ptr` → `file_cache_ex_ptr` +3. 处理 `prepend_header()` 的 `bool` 返回值 +4. 可选：利用 `SetMaxHeaderLength()` / `SetMaxFileSize()` 配置参数 + +## 线程安全说明 + +- `FileCacheEx` 继承 `hv::LRUCache` 的全局互斥锁保护 LRU 操作 +- 每个 `file_cache_ex_s` 条目内置 `std::mutex` 保护条目级修改 +- `prepend_header()` 自动加锁，调用方无需额外同步 +- `is_modified()` / `is_complete()` / `resize_buf()` 需要调用方持锁 diff --git a/http/server/FileCacheEx.cpp b/http/server/FileCacheEx.cpp index 30728d8dd..875497dfa 100644 --- a/http/server/FileCacheEx.cpp +++ b/http/server/FileCacheEx.cpp @@ -5,11 +5,11 @@ #include "htime.h" #include "hlog.h" -#include "httpdef.h" // http_content_type_str_by_suffix -#include "http_page.h" // make_index_of_page +#include "httpdef.h" // import http_content_type_str_by_suffix +#include "http_page.h" // import make_index_of_page #ifdef OS_WIN -#include "hstring.h" // hv::utf8_to_wchar +#include "hstring.h" // import hv::utf8_to_wchar #endif #define ETAG_FMT "\"%zx-%zx\"" From f5e9a67b206a6a7f765612c161eaf3b56efb0d21 Mon Sep 17 00:00:00 2001 From: Yundi339 Date: Thu, 26 Mar 2026 23:07:29 +0800 Subject: [PATCH 05/20] fix: add bounds check for negative reserved in resize_buf Prevent integer overflow when reserved < 0 is passed to resize_buf(), which would cause (size_t)reserved to become a very large value. --- http/server/FileCacheEx.h | 1 + 1 file changed, 1 insertion(+) diff --git a/http/server/FileCacheEx.h b/http/server/FileCacheEx.h index f6c3af5b7..c4ce17c75 100644 --- a/http/server/FileCacheEx.h +++ b/http/server/FileCacheEx.h @@ -73,6 +73,7 @@ typedef struct file_cache_ex_s { // NOTE: caller must hold mutex — invalidates filebuf/httpbuf pointers void resize_buf(size_t filesize, int reserved) { + if (reserved < 0) reserved = 0; header_reserve = reserved; buf.resize((size_t)reserved + filesize); filebuf.base = buf.base + reserved; From 7c7d63ee61ab2b8ef1abab69c18d1210e53ecccc Mon Sep 17 00:00:00 2001 From: Yundi339 Date: Sun, 29 Mar 2026 16:51:29 +0800 Subject: [PATCH 06/20] refactor: replace FileCache with FileCacheEx in HttpHandler and HttpServer --- http/server/HttpHandler.cpp | 12 ++++++------ http/server/HttpHandler.h | 6 +++--- http/server/HttpServer.cpp | 10 +++++----- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/http/server/HttpHandler.cpp b/http/server/HttpHandler.cpp index 005967328..c618d1287 100644 --- a/http/server/HttpHandler.cpp +++ b/http/server/HttpHandler.cpp @@ -608,8 +608,8 @@ int HttpHandler::defaultStaticHandler() { return status_code; } - // FileCache - FileCache::OpenParam param; + // FileCacheEx + FileCacheEx::OpenParam param; param.max_read = service->max_file_cache_size; param.need_read = !(req->method == HTTP_HEAD || has_range); param.path = req_path; @@ -689,7 +689,7 @@ int HttpHandler::defaultErrorHandler() { std::string filepath = service->document_root + '/' + service->error_page; if (files) { // cache and load error page - FileCache::OpenParam param; + FileCacheEx::OpenParam param; fc = files->Open(filepath.c_str(), ¶m); } } @@ -798,7 +798,7 @@ int HttpHandler::GetSendData(char** data, size_t* len) { } // File service if (fc) { - // FileCache + // FileCacheEx // NOTE: no copy filebuf, more efficient header = pResp->Dump(true, false); fc->prepend_header(header.c_str(), header.size()); @@ -842,8 +842,8 @@ int HttpHandler::GetSendData(char** data, size_t* len) { } case SEND_DONE: { - // NOTE: remove file cache if > FILE_CACHE_MAX_SIZE - if (fc && fc->filebuf.len > FILE_CACHE_MAX_SIZE) { + // NOTE: remove file cache if > max_file_size + if (fc && fc->filebuf.len > files->GetMaxFileSize()) { files->Close(fc->filepath.c_str()); } fc = NULL; diff --git a/http/server/HttpHandler.h b/http/server/HttpHandler.h index b72f07772..5dc0b5261 100644 --- a/http/server/HttpHandler.h +++ b/http/server/HttpHandler.h @@ -3,7 +3,7 @@ #include "HttpService.h" #include "HttpParser.h" -#include "FileCache.h" +#include "FileCacheEx.h" #include "WebSocketServer.h" #include "WebSocketParser.h" @@ -71,8 +71,8 @@ class HttpHandler { uint64_t last_recv_pong_time; // for sendfile - FileCache *files; - file_cache_ptr fc; // cache small file + FileCacheEx *files; + file_cache_ex_ptr fc; // cache small file struct LargeFile : public HFile { HBuf buf; uint64_t timer; diff --git a/http/server/HttpServer.cpp b/http/server/HttpServer.cpp index aa296fe8b..9d19708e2 100644 --- a/http/server/HttpServer.cpp +++ b/http/server/HttpServer.cpp @@ -19,7 +19,7 @@ struct HttpServerPrivdata { std::vector threads; std::mutex mutex_; std::shared_ptr service; - FileCache filecache; + FileCacheEx filecache; }; static void on_recv(hio_t* io, void* buf, int readbytes) { @@ -86,7 +86,7 @@ static void on_accept(hio_t* io) { handler->service = service; // websocket service handler->ws_service = server->ws; - // FileCache + // FileCacheEx HttpServerPrivdata* privdata = (HttpServerPrivdata*)server->privdata; handler->files = &privdata->filecache; hevent_set_userdata(io, handler); @@ -132,14 +132,14 @@ static void loop_thread(void* userdata) { service->Static("/", service->document_root.c_str()); } - // FileCache - FileCache* filecache = &privdata->filecache; + // FileCacheEx + FileCacheEx* filecache = &privdata->filecache; filecache->stat_interval = service->file_cache_stat_interval; filecache->expired_time = service->file_cache_expired_time; if (filecache->expired_time > 0) { // NOTE: add timer to remove expired file cache htimer_t* timer = htimer_add(hloop, [](htimer_t* timer) { - FileCache* filecache = (FileCache*)hevent_userdata(timer); + FileCacheEx* filecache = (FileCacheEx*)hevent_userdata(timer); filecache->RemoveExpiredFileCache(); }, filecache->expired_time * 1000); hevent_set_userdata(timer, filecache); From a9790f60ec5740d3874c5e925f29d4718ccbcf11 Mon Sep 17 00:00:00 2001 From: Yundi339 Date: Mon, 6 Apr 2026 00:15:18 +0800 Subject: [PATCH 07/20] Refactor FileCache and remove FileCacheEx --- cmake/vars.cmake | 2 +- docs/FileCacheEx.md | 119 ------------------------ docs/cn/FileCacheEx.md | 134 --------------------------- http/server/FileCache.cpp | 82 +++++++++++------ http/server/FileCache.h | 116 ++++++++++++++++++------ http/server/FileCacheEx.cpp | 174 ------------------------------------ http/server/FileCacheEx.h | 157 -------------------------------- http/server/HttpHandler.cpp | 8 +- http/server/HttpHandler.h | 6 +- http/server/HttpServer.cpp | 10 +-- 10 files changed, 156 insertions(+), 652 deletions(-) delete mode 100644 docs/FileCacheEx.md delete mode 100644 docs/cn/FileCacheEx.md delete mode 100644 http/server/FileCacheEx.cpp delete mode 100644 http/server/FileCacheEx.h diff --git a/cmake/vars.cmake b/cmake/vars.cmake index 5276bb297..a425a8c75 100644 --- a/cmake/vars.cmake +++ b/cmake/vars.cmake @@ -103,7 +103,7 @@ set(HTTP_SERVER_HEADERS http/server/HttpContext.h http/server/HttpResponseWriter.h http/server/WebSocketServer.h - http/server/FileCacheEx.h + http/server/FileCache.h ) set(MQTT_HEADERS diff --git a/docs/FileCacheEx.md b/docs/FileCacheEx.md deleted file mode 100644 index 622c56c6d..000000000 --- a/docs/FileCacheEx.md +++ /dev/null @@ -1,119 +0,0 @@ -# FileCacheEx — Enhanced File Cache - -`FileCacheEx` is a drop-in replacement for `FileCache` that fixes thread-safety issues and provides runtime-configurable parameters. - -## Header - -```c++ -#include "FileCacheEx.h" -``` - -## Improvements over FileCache - -| Feature | FileCache | FileCacheEx | -|---------|-----------|-------------| -| HTTP header reserve | Compile-time 1024 bytes | Runtime configurable, default 4096 | -| `prepend_header()` | Returns `void`, silently drops overflow | Returns `bool`, `false` on overflow | -| Per-entry thread safety | No locking | Per-entry `std::mutex` | -| `resize_buf()` httpbuf | May leave dangling reference | Explicitly invalidated (UAF prevention) | -| File read | Single `read()`, may short-read | Loop with `EINTR` handling | -| Cache insertion | Before initialization completes | Deferred until fully initialized | -| Max file size | Compile-time `FILE_CACHE_MAX_SIZE` | Runtime `SetMaxFileSize()` | -| DLL export | None | `HV_EXPORT` | - -## Data Structures - -```c++ -typedef struct file_cache_ex_s { - mutable std::mutex mutex; // per-entry lock - std::string filepath; - struct stat st; - time_t open_time; - time_t stat_time; - uint32_t stat_cnt; - HBuf buf; // header_reserve + file_content - hbuf_t filebuf; // points into buf: file content - hbuf_t httpbuf; // points into buf: header + content - char last_modified[64]; - char etag[64]; - std::string content_type; - int header_reserve; // bytes reserved before content - int header_used; // bytes actually used by header - - bool is_modified(); // caller must hold mutex - bool is_complete(); // caller must hold mutex - void resize_buf(size_t filesize, int reserved); // caller must hold mutex - void resize_buf(size_t filesize); - bool prepend_header(const char* header, int len); // thread-safe - - // Thread-safe accessors - int get_header_reserve() const; - int get_header_used() const; - int get_header_remaining() const; - bool header_fits(int len) const; -} file_cache_ex_t; - -typedef std::shared_ptr file_cache_ex_ptr; -``` - -## FileCacheEx Class - -```c++ -class HV_EXPORT FileCacheEx : public hv::LRUCache { -public: - int stat_interval; // seconds between stat() checks, default 10 - int expired_time; // seconds before expiry, default 60 - int max_header_length; // header reserve per entry, default 4096 - int max_file_size; // max cached file size, default 4MB - - explicit FileCacheEx(size_t capacity = 100); - - file_cache_ex_ptr Open(const char* filepath, OpenParam* param); - bool Exists(const char* filepath) const; - bool Close(const char* filepath); - void RemoveExpiredFileCache(); - - int GetMaxHeaderLength() const; - int GetMaxFileSize() const; - int GetStatInterval() const; - int GetExpiredTime() const; - - void SetMaxHeaderLength(int len); - void SetMaxFileSize(int size); -}; -``` - -## Usage - -```c++ -FileCacheEx filecache(200); // LRU capacity = 200 -filecache.SetMaxHeaderLength(8192); // 8K header reserve -filecache.SetMaxFileSize(1 << 24); // 16MB max file -filecache.stat_interval = 5; -filecache.expired_time = 120; - -FileCacheEx::OpenParam param; -file_cache_ex_ptr fc = filecache.Open("/var/www/index.html", ¶m); -if (fc) { - std::string header = "HTTP/1.1 200 OK\r\nContent-Type: text/html\r\n\r\n"; - if (fc->prepend_header(header.c_str(), header.size())) { - // send fc->httpbuf (header + content) - } else { - // header exceeds reserve, fall back to separate send - } -} -``` - -## Migration from FileCache - -1. Replace `#include "FileCache.h"` with `#include "FileCacheEx.h"` -2. Change types: `FileCache` → `FileCacheEx`, `file_cache_ptr` → `file_cache_ex_ptr` -3. Handle `bool` return from `prepend_header()` -4. Optional: configure via `SetMaxHeaderLength()` / `SetMaxFileSize()` - -## Thread Safety - -- `FileCacheEx` inherits `hv::LRUCache` global mutex for LRU operations -- Each `file_cache_ex_s` entry has its own `std::mutex` for entry-level protection -- `prepend_header()` locks automatically; callers need no external synchronization -- `is_modified()`, `is_complete()`, `resize_buf()` require caller to hold the mutex diff --git a/docs/cn/FileCacheEx.md b/docs/cn/FileCacheEx.md deleted file mode 100644 index f99cb04cf..000000000 --- a/docs/cn/FileCacheEx.md +++ /dev/null @@ -1,134 +0,0 @@ -# FileCacheEx — 增强版文件缓存 - -`FileCacheEx` 是 `FileCache` 的增强替代方案，解决了原始实现中的线程安全问题，并提供运行时可配置的参数。 - -## 头文件 - -```c++ -#include "FileCacheEx.h" -``` - -## 与 FileCache 的区别 - -| 特性 | FileCache | FileCacheEx | -|------|-----------|-------------| -| HTTP 头部预留空间 | 编译期固定 1024 字节 | 运行时可配置，默认 4096 字节 | -| `prepend_header()` | 返回 `void`，溢出静默丢弃 | 返回 `bool`，失败返回 `false` | -| 缓存条目线程安全 | 无锁保护 | 每条目 `std::mutex` | -| `resize_buf()` 后 httpbuf | 可能悬空引用 | 主动置空，防止 UAF | -| 文件读取 | 单次 `read()`，可能短读 | 循环读取，处理 `EINTR` | -| `put()` 时机 | 初始化前放入缓存 | 完全初始化后放入缓存 | -| 最大文件大小 | 编译期宏 `FILE_CACHE_MAX_SIZE` | 运行时 `SetMaxFileSize()` | -| DLL 导出 | 无 | `HV_EXPORT` | - -## 数据结构 - -```c++ -// 缓存条目 -typedef struct file_cache_ex_s { - mutable std::mutex mutex; // 条目级互斥锁 - std::string filepath; - struct stat st; - time_t open_time; - time_t stat_time; - uint32_t stat_cnt; - HBuf buf; // header_reserve + file_content - hbuf_t filebuf; // 指向 buf 中文件内容区域 - hbuf_t httpbuf; // 指向 buf 中 header + 文件内容区域 - char last_modified[64]; - char etag[64]; - std::string content_type; - int header_reserve; // 头部预留字节数 - int header_used; // 实际使用的头部字节数 - - // 方法 - bool is_modified(); // 检查文件是否被修改（需持锁） - bool is_complete(); // 检查缓存是否完整（需持锁） - void resize_buf(size_t filesize, int reserved); // 重新分配缓冲区（需持锁） - void resize_buf(size_t filesize); // 使用当前 header_reserve - bool prepend_header(const char* header, int len); // 线程安全：写入 HTTP 头 - - // 线程安全访问器 - int get_header_reserve() const; - int get_header_used() const; - int get_header_remaining() const; - bool header_fits(int len) const; -} file_cache_ex_t; - -typedef std::shared_ptr file_cache_ex_ptr; -``` - -## FileCacheEx 类 - -```c++ -class HV_EXPORT FileCacheEx : public hv::LRUCache { -public: - // 可配置参数 - int stat_interval; // stat() 检查间隔（秒），默认 10 - int expired_time; // 缓存过期时间（秒），默认 60 - int max_header_length; // 每条目头部预留字节，默认 4096 - int max_file_size; // 最大缓存文件大小，默认 4MB - - explicit FileCacheEx(size_t capacity = 100); - - // 打开文件并缓存 - file_cache_ex_ptr Open(const char* filepath, OpenParam* param); - - // 检查文件是否在缓存中 - bool Exists(const char* filepath) const; - - // 从缓存中移除文件 - bool Close(const char* filepath); - - // 移除过期缓存条目 - void RemoveExpiredFileCache(); - - // Getter - int GetMaxHeaderLength() const; - int GetMaxFileSize() const; - int GetStatInterval() const; - int GetExpiredTime() const; - - // Setter - void SetMaxHeaderLength(int len); - void SetMaxFileSize(int size); -}; -``` - -## 使用示例 - -```c++ -FileCacheEx filecache(200); // 最大 200 个缓存条目 -filecache.SetMaxHeaderLength(8192); // 8K 头部预留 -filecache.SetMaxFileSize(1 << 24); // 16MB 最大缓存文件 -filecache.stat_interval = 5; // 每 5 秒检查文件变更 -filecache.expired_time = 120; // 2 分钟过期 - -FileCacheEx::OpenParam param; -file_cache_ex_ptr fc = filecache.Open("/var/www/index.html", ¶m); -if (fc) { - // 写入 HTTP 响应头 - std::string header = "HTTP/1.1 200 OK\r\nContent-Type: text/html\r\n\r\n"; - if (fc->prepend_header(header.c_str(), header.size())) { - // 发送 fc->httpbuf (header + content) - } else { - // 头部超出预留空间，回退到普通发送 - } -} -``` - -## 迁移指南 - -从 `FileCache` 迁移到 `FileCacheEx`： - -1. 替换头文件引用：`#include "FileCache.h"` → `#include "FileCacheEx.h"` -2. 替换类型：`FileCache` → `FileCacheEx`，`file_cache_ptr` → `file_cache_ex_ptr` -3. 处理 `prepend_header()` 的 `bool` 返回值 -4. 可选：利用 `SetMaxHeaderLength()` / `SetMaxFileSize()` 配置参数 - -## 线程安全说明 - -- `FileCacheEx` 继承 `hv::LRUCache` 的全局互斥锁保护 LRU 操作 -- 每个 `file_cache_ex_s` 条目内置 `std::mutex` 保护条目级修改 -- `prepend_header()` 自动加锁，调用方无需额外同步 -- `is_modified()` / `is_complete()` / `resize_buf()` 需要调用方持锁 diff --git a/http/server/FileCache.cpp b/http/server/FileCache.cpp index 26b76e150..e8bccd69b 100644 --- a/http/server/FileCache.cpp +++ b/http/server/FileCache.cpp @@ -5,18 +5,21 @@ #include "htime.h" #include "hlog.h" -#include "httpdef.h" // import http_content_type_str_by_suffix +#include "httpdef.h" // import http_content_type_str_by_suffix #include "http_page.h" // import make_index_of_page #ifdef OS_WIN -#include "hstring.h" // import hv::utf8_to_wchar +#include "hstring.h" // import hv::utf8_to_wchar #endif #define ETAG_FMT "\"%zx-%zx\"" -FileCache::FileCache(size_t capacity) : hv::LRUCache(capacity) { - stat_interval = 10; // s - expired_time = 60; // s +FileCache::FileCache(size_t capacity) + : hv::LRUCache(capacity) { + stat_interval = 10; // s + expired_time = 60; // s + max_header_length = FILE_CACHE_DEFAULT_HEADER_LENGTH; + max_file_size = FILE_CACHE_DEFAULT_MAX_FILE_SIZE; } file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) { @@ -26,6 +29,7 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) { #endif bool modified = false; if (fc) { + std::lock_guard lock(fc->mutex); time_t now = time(NULL); if (now - fc->stat_time > stat_interval) { fc->stat_time = now; @@ -53,19 +57,18 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) { #endif int fd = -1; #ifdef OS_WIN - if(wfilepath.empty()) wfilepath = hv::utf8_to_wchar(filepath); - if(_wstat(wfilepath.c_str(), (struct _stat*)&st) != 0) { + if (wfilepath.empty()) wfilepath = hv::utf8_to_wchar(filepath); + if (_wstat(wfilepath.c_str(), (struct _stat*)&st) != 0) { param->error = ERR_OPEN_FILE; return NULL; } - if(S_ISREG(st.st_mode)) { + if (S_ISREG(st.st_mode)) { fd = _wopen(wfilepath.c_str(), flags); - }else if (S_ISDIR(st.st_mode)) { - // NOTE: open(dir) return -1 on windows + } else if (S_ISDIR(st.st_mode)) { fd = 0; } #else - if(stat(filepath, &st) != 0) { + if (::stat(filepath, &st) != 0) { param->error = ERR_OPEN_FILE; return NULL; } @@ -75,62 +78,84 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) { param->error = ERR_OPEN_FILE; return NULL; } - defer(if (fd > 0) { close(fd); }) +#ifdef OS_WIN + defer(if (fd > 0) { close(fd); }) // fd=0 is Windows directory sentinel +#else + defer(close(fd);) +#endif if (fc == NULL) { if (S_ISREG(st.st_mode) || (S_ISDIR(st.st_mode) && - filepath[strlen(filepath)-1] == '/')) { + filepath[strlen(filepath) - 1] == '/')) { fc = std::make_shared(); fc->filepath = filepath; fc->st = st; + fc->header_reserve = max_header_length; time(&fc->open_time); fc->stat_time = fc->open_time; fc->stat_cnt = 1; - put(filepath, fc); - } - else { + // NOTE: do NOT put() into cache yet — defer until fully initialized + } else { param->error = ERR_MISMATCH; return NULL; } } + // Hold fc->mutex for the remainder of initialization + std::lock_guard lock(fc->mutex); if (S_ISREG(fc->st.st_mode)) { param->filesize = fc->st.st_size; // FILE if (param->need_read) { if (fc->st.st_size > param->max_read) { param->error = ERR_OVER_LIMIT; + // Don't cache incomplete entries return NULL; } - fc->resize_buf(fc->st.st_size); - int nread = read(fd, fc->filebuf.base, fc->filebuf.len); - if (nread != fc->filebuf.len) { - hloge("Failed to read file: %s", filepath); - param->error = ERR_READ_FILE; - return NULL; + fc->resize_buf(fc->st.st_size, max_header_length); + // Loop to handle partial reads (EINTR, etc.) + char* dst = fc->filebuf.base; + size_t remaining = fc->filebuf.len; + while (remaining > 0) { + ssize_t nread = read(fd, dst, remaining); + if (nread < 0) { + if (errno == EINTR) continue; + hloge("Failed to read file: %s", filepath); + param->error = ERR_READ_FILE; + return NULL; + } + if (nread == 0) { + hloge("Unexpected EOF reading file: %s", filepath); + param->error = ERR_READ_FILE; + return NULL; + } + dst += nread; + remaining -= nread; } } const char* suffix = strrchr(filepath, '.'); if (suffix) { - http_content_type content_type = http_content_type_enum_by_suffix(suffix+1); + http_content_type content_type = http_content_type_enum_by_suffix(suffix + 1); if (content_type == TEXT_HTML) { fc->content_type = "text/html; charset=utf-8"; } else if (content_type == TEXT_PLAIN) { fc->content_type = "text/plain; charset=utf-8"; } else { - fc->content_type = http_content_type_str_by_suffix(suffix+1); + fc->content_type = http_content_type_str_by_suffix(suffix + 1); } } - } - else if (S_ISDIR(fc->st.st_mode)) { + } else if (S_ISDIR(fc->st.st_mode)) { // DIR std::string page; make_index_of_page(filepath, page, param->path); - fc->resize_buf(page.size()); + fc->resize_buf(page.size(), max_header_length); memcpy(fc->filebuf.base, page.c_str(), page.size()); fc->content_type = "text/html; charset=utf-8"; } gmtime_fmt(fc->st.st_mtime, fc->last_modified); - snprintf(fc->etag, sizeof(fc->etag), ETAG_FMT, (size_t)fc->st.st_mtime, (size_t)fc->st.st_size); + snprintf(fc->etag, sizeof(fc->etag), ETAG_FMT, + (size_t)fc->st.st_mtime, (size_t)fc->st.st_size); + // Cache the fully initialized entry + put(filepath, fc); } return fc; } @@ -154,6 +179,7 @@ file_cache_ptr FileCache::Get(const char* filepath) { void FileCache::RemoveExpiredFileCache() { time_t now = time(NULL); remove_if([this, now](const std::string& filepath, const file_cache_ptr& fc) { + std::lock_guard lock(fc->mutex); return (now - fc->stat_time > expired_time); }); } diff --git a/http/server/FileCache.h b/http/server/FileCache.h index 363c41d88..c43ce1943 100644 --- a/http/server/FileCache.h +++ b/http/server/FileCache.h @@ -1,90 +1,152 @@ #ifndef HV_FILE_CACHE_H_ #define HV_FILE_CACHE_H_ +/* + * FileCache — Enhanced File Cache for libhv HTTP server + * + * Features: + * 1. Configurable max_header_length (default 4096, tunable per-instance) + * 2. prepend_header() returns bool to report success/failure + * 3. Exposes header/buffer metrics via accessors + * 4. Fixes stat() name collision in is_modified() + * 5. max_cache_num / max_file_size configurable at runtime + * 6. Reserved header space can be tuned per-instance + * 7. Source-level API compatible; struct layout differs from original (no ABI/layout compatibility) + */ + #include -#include #include #include +#include "hexport.h" #include "hbuf.h" #include "hstring.h" #include "LRUCache.h" -#define HTTP_HEADER_MAX_LENGTH 1024 // 1K -#define FILE_CACHE_MAX_NUM 100 -#define FILE_CACHE_MAX_SIZE (1 << 22) // 4M +// Default values — may be overridden at runtime via FileCache setters +#define FILE_CACHE_DEFAULT_HEADER_LENGTH 4096 // 4K +#define FILE_CACHE_DEFAULT_MAX_NUM 100 +#define FILE_CACHE_DEFAULT_MAX_FILE_SIZE (1 << 22) // 4M typedef struct file_cache_s { + mutable std::mutex mutex; // protects all mutable state below std::string filepath; struct stat st; time_t open_time; time_t stat_time; uint32_t stat_cnt; - HBuf buf; // http_header + file_content - hbuf_t filebuf; - hbuf_t httpbuf; + HBuf buf; // header_reserve + file_content + hbuf_t filebuf; // points into buf: file content region + hbuf_t httpbuf; // points into buf: header + file content after prepend char last_modified[64]; char etag[64]; std::string content_type; + // --- new: expose header metrics --- + int header_reserve; // reserved bytes before file content + int header_used; // actual bytes used by prepend_header + file_cache_s() { stat_cnt = 0; + header_reserve = FILE_CACHE_DEFAULT_HEADER_LENGTH; + header_used = 0; + memset(last_modified, 0, sizeof(last_modified)); + memset(etag, 0, sizeof(etag)); } + // NOTE: caller must hold mutex. + // On Windows, Open() uses _wstat() directly instead of calling this. bool is_modified() { time_t mtime = st.st_mtime; - stat(filepath.c_str(), &st); + ::stat(filepath.c_str(), &st); return mtime != st.st_mtime; } + // NOTE: caller must hold mutex bool is_complete() { - if(S_ISDIR(st.st_mode)) return filebuf.len > 0; - return filebuf.len == st.st_size; + if (S_ISDIR(st.st_mode)) return filebuf.len > 0; + return filebuf.len == (size_t)st.st_size; } - void resize_buf(int filesize) { - buf.resize(HTTP_HEADER_MAX_LENGTH + filesize); - filebuf.base = buf.base + HTTP_HEADER_MAX_LENGTH; + // NOTE: caller must hold mutex — invalidates filebuf/httpbuf pointers + void resize_buf(size_t filesize, int reserved) { + if (reserved < 0) reserved = 0; + header_reserve = reserved; + buf.resize((size_t)reserved + filesize); + filebuf.base = buf.base + reserved; filebuf.len = filesize; + // Invalidate httpbuf since buffer may have been reallocated + httpbuf.base = NULL; + httpbuf.len = 0; + header_used = 0; + } + + void resize_buf(size_t filesize) { + resize_buf(filesize, header_reserve); } - void prepend_header(const char* header, int len) { - if (len > HTTP_HEADER_MAX_LENGTH) return; + // Thread-safe: prepend header into reserved space. + // Returns true on success, false if header exceeds reserved space. + bool prepend_header(const char* header, int len) { + std::lock_guard lock(mutex); + if (len <= 0 || len > header_reserve) return false; httpbuf.base = filebuf.base - len; - httpbuf.len = len + filebuf.len; + httpbuf.len = (size_t)len + filebuf.len; memcpy(httpbuf.base, header, len); + header_used = len; + return true; } + + // --- thread-safe accessors --- + int get_header_reserve() const { std::lock_guard lock(mutex); return header_reserve; } + int get_header_used() const { std::lock_guard lock(mutex); return header_used; } + int get_header_remaining() const { std::lock_guard lock(mutex); return header_reserve - header_used; } + bool header_fits(int len) const { std::lock_guard lock(mutex); return len > 0 && len <= header_reserve; } } file_cache_t; -typedef std::shared_ptr file_cache_ptr; +typedef std::shared_ptr file_cache_ptr; -class FileCache : public hv::LRUCache { +class HV_EXPORT FileCache : public hv::LRUCache { public: - int stat_interval; - int expired_time; + // --- configurable parameters (were hardcoded macros before) --- + int stat_interval; // seconds between stat() checks + int expired_time; // seconds before cache entry expires + int max_header_length; // reserved header bytes per entry + int max_file_size; // max cached file size (larger = large-file path) - FileCache(size_t capacity = FILE_CACHE_MAX_NUM); + explicit FileCache(size_t capacity = FILE_CACHE_DEFAULT_MAX_NUM); struct OpenParam { - bool need_read; - int max_read; - const char* path; - size_t filesize; - int error; + bool need_read; + int max_read; // per-request override for max file size + const char* path; // URL path (for directory listing) + size_t filesize; // [out] actual file size + int error; // [out] error code if Open returns NULL OpenParam() { need_read = true; - max_read = FILE_CACHE_MAX_SIZE; + max_read = FILE_CACHE_DEFAULT_MAX_FILE_SIZE; path = "/"; filesize = 0; error = 0; } }; + file_cache_ptr Open(const char* filepath, OpenParam* param); bool Exists(const char* filepath) const; bool Close(const char* filepath); void RemoveExpiredFileCache(); + // --- new: getters --- + int GetMaxHeaderLength() const { return max_header_length; } + int GetMaxFileSize() const { return max_file_size; } + int GetStatInterval() const { return stat_interval; } + int GetExpiredTime() const { return expired_time; } + + // --- new: setters --- + void SetMaxHeaderLength(int len) { max_header_length = len; } + void SetMaxFileSize(int size) { max_file_size = size; } + protected: file_cache_ptr Get(const char* filepath); }; diff --git a/http/server/FileCacheEx.cpp b/http/server/FileCacheEx.cpp deleted file mode 100644 index 875497dfa..000000000 --- a/http/server/FileCacheEx.cpp +++ /dev/null @@ -1,174 +0,0 @@ -#include "FileCacheEx.h" - -#include "herr.h" -#include "hscope.h" -#include "htime.h" -#include "hlog.h" - -#include "httpdef.h" // import http_content_type_str_by_suffix -#include "http_page.h" // import make_index_of_page - -#ifdef OS_WIN -#include "hstring.h" // import hv::utf8_to_wchar -#endif - -#define ETAG_FMT "\"%zx-%zx\"" - -FileCacheEx::FileCacheEx(size_t capacity) - : hv::LRUCache(capacity) { - stat_interval = 10; // s - expired_time = 60; // s - max_header_length = FILECACHE_EX_DEFAULT_HEADER_LENGTH; - max_file_size = FILECACHE_EX_DEFAULT_MAX_FILE_SIZE; -} - -file_cache_ex_ptr FileCacheEx::Open(const char* filepath, OpenParam* param) { - file_cache_ex_ptr fc = Get(filepath); -#ifdef OS_WIN - std::wstring wfilepath; -#endif - bool modified = false; - if (fc) { - std::lock_guard lock(fc->mutex); - time_t now = time(NULL); - if (now - fc->stat_time > stat_interval) { - fc->stat_time = now; - fc->stat_cnt++; -#ifdef OS_WIN - wfilepath = hv::utf8_to_wchar(filepath); - now = fc->st.st_mtime; - _wstat(wfilepath.c_str(), (struct _stat*)&fc->st); - modified = now != fc->st.st_mtime; -#else - modified = fc->is_modified(); -#endif - } - if (param->need_read) { - if (!modified && fc->is_complete()) { - param->need_read = false; - } - } - } - if (fc == NULL || modified || param->need_read) { - struct stat st; - int flags = O_RDONLY; -#ifdef O_BINARY - flags |= O_BINARY; -#endif - int fd = -1; -#ifdef OS_WIN - if (wfilepath.empty()) wfilepath = hv::utf8_to_wchar(filepath); - if (_wstat(wfilepath.c_str(), (struct _stat*)&st) != 0) { - param->error = ERR_OPEN_FILE; - return NULL; - } - if (S_ISREG(st.st_mode)) { - fd = _wopen(wfilepath.c_str(), flags); - } else if (S_ISDIR(st.st_mode)) { - fd = 0; - } -#else - if (::stat(filepath, &st) != 0) { - param->error = ERR_OPEN_FILE; - return NULL; - } - fd = open(filepath, flags); -#endif - if (fd < 0) { - param->error = ERR_OPEN_FILE; - return NULL; - } - defer(if (fd > 0) { close(fd); }) - if (fc == NULL) { - if (S_ISREG(st.st_mode) || - (S_ISDIR(st.st_mode) && - filepath[strlen(filepath) - 1] == '/')) { - fc = std::make_shared(); - fc->filepath = filepath; - fc->st = st; - fc->header_reserve = max_header_length; - time(&fc->open_time); - fc->stat_time = fc->open_time; - fc->stat_cnt = 1; - // NOTE: do NOT put() into cache yet — defer until fully initialized - } else { - param->error = ERR_MISMATCH; - return NULL; - } - } - // Hold fc->mutex for the remainder of initialization - std::lock_guard lock(fc->mutex); - if (S_ISREG(fc->st.st_mode)) { - param->filesize = fc->st.st_size; - // FILE - if (param->need_read) { - if (fc->st.st_size > param->max_read) { - param->error = ERR_OVER_LIMIT; - // Don't cache incomplete entries - return NULL; - } - fc->resize_buf(fc->st.st_size, max_header_length); - // Loop to handle partial reads (EINTR, etc.) - char* dst = fc->filebuf.base; - size_t remaining = fc->filebuf.len; - while (remaining > 0) { - int nread = read(fd, dst, remaining); - if (nread <= 0) { - hloge("Failed to read file: %s", filepath); - param->error = ERR_READ_FILE; - return NULL; - } - dst += nread; - remaining -= nread; - } - } - const char* suffix = strrchr(filepath, '.'); - if (suffix) { - http_content_type content_type = http_content_type_enum_by_suffix(suffix + 1); - if (content_type == TEXT_HTML) { - fc->content_type = "text/html; charset=utf-8"; - } else if (content_type == TEXT_PLAIN) { - fc->content_type = "text/plain; charset=utf-8"; - } else { - fc->content_type = http_content_type_str_by_suffix(suffix + 1); - } - } - } else if (S_ISDIR(fc->st.st_mode)) { - // DIR - std::string page; - make_index_of_page(filepath, page, param->path); - fc->resize_buf(page.size(), max_header_length); - memcpy(fc->filebuf.base, page.c_str(), page.size()); - fc->content_type = "text/html; charset=utf-8"; - } - gmtime_fmt(fc->st.st_mtime, fc->last_modified); - snprintf(fc->etag, sizeof(fc->etag), ETAG_FMT, - (size_t)fc->st.st_mtime, (size_t)fc->st.st_size); - // Cache the fully initialized entry - put(filepath, fc); - } - return fc; -} - -bool FileCacheEx::Exists(const char* filepath) const { - return contains(filepath); -} - -bool FileCacheEx::Close(const char* filepath) { - return remove(filepath); -} - -file_cache_ex_ptr FileCacheEx::Get(const char* filepath) { - file_cache_ex_ptr fc; - if (get(filepath, fc)) { - return fc; - } - return NULL; -} - -void FileCacheEx::RemoveExpiredFileCache() { - time_t now = time(NULL); - remove_if([this, now](const std::string& filepath, const file_cache_ex_ptr& fc) { - return (now - fc->stat_time > expired_time); - }); -} diff --git a/http/server/FileCacheEx.h b/http/server/FileCacheEx.h deleted file mode 100644 index c4ce17c75..000000000 --- a/http/server/FileCacheEx.h +++ /dev/null @@ -1,157 +0,0 @@ -#ifndef HV_FILE_CACHE_EX_H_ -#define HV_FILE_CACHE_EX_H_ - -/* - * FileCacheEx — Enhanced File Cache for libhv HTTP server - * - * Improvements over the original FileCache: - * 1. Configurable max_header_length (no more hardcoded 4096) - * 2. prepend_header() returns bool to report success/failure - * 3. Exposes header/buffer metrics via accessors - * 4. Fixes stat() name collision in is_modified() - * 5. max_cache_num / max_file_size configurable at runtime - * 6. Reserved header space can be tuned per-instance - * 7. Fully backward-compatible struct layout - * - * This is a NEW module alongside FileCache — no modifications to the - * original code — to keep things non-invasive for upstream PR review. - */ - -#include -#include -#include - -#include "hexport.h" -#include "hbuf.h" -#include "hstring.h" -#include "LRUCache.h" - -// Default values — may be overridden at runtime via FileCacheEx setters -#define FILECACHE_EX_DEFAULT_HEADER_LENGTH 4096 // 4K -#define FILECACHE_EX_DEFAULT_MAX_NUM 100 -#define FILECACHE_EX_DEFAULT_MAX_FILE_SIZE (1 << 22) // 4M - -typedef struct file_cache_ex_s { - mutable std::mutex mutex; // protects all mutable state below - std::string filepath; - struct stat st; - time_t open_time; - time_t stat_time; - uint32_t stat_cnt; - HBuf buf; // header_reserve + file_content - hbuf_t filebuf; // points into buf: file content region - hbuf_t httpbuf; // points into buf: header + file content after prepend - char last_modified[64]; - char etag[64]; - std::string content_type; - - // --- new: expose header metrics --- - int header_reserve; // reserved bytes before file content - int header_used; // actual bytes used by prepend_header - - file_cache_ex_s() { - stat_cnt = 0; - header_reserve = FILECACHE_EX_DEFAULT_HEADER_LENGTH; - header_used = 0; - memset(last_modified, 0, sizeof(last_modified)); - memset(etag, 0, sizeof(etag)); - } - - // NOTE: caller must hold mutex. - // On Windows, Open() uses _wstat() directly instead of calling this. - bool is_modified() { - time_t mtime = st.st_mtime; - ::stat(filepath.c_str(), &st); - return mtime != st.st_mtime; - } - - // NOTE: caller must hold mutex - bool is_complete() { - if (S_ISDIR(st.st_mode)) return filebuf.len > 0; - return filebuf.len == (size_t)st.st_size; - } - - // NOTE: caller must hold mutex — invalidates filebuf/httpbuf pointers - void resize_buf(size_t filesize, int reserved) { - if (reserved < 0) reserved = 0; - header_reserve = reserved; - buf.resize((size_t)reserved + filesize); - filebuf.base = buf.base + reserved; - filebuf.len = filesize; - // Invalidate httpbuf since buffer may have been reallocated - httpbuf.base = NULL; - httpbuf.len = 0; - header_used = 0; - } - - void resize_buf(size_t filesize) { - resize_buf(filesize, header_reserve); - } - - // Thread-safe: prepend header into reserved space. - // Returns true on success, false if header exceeds reserved space. - bool prepend_header(const char* header, int len) { - std::lock_guard lock(mutex); - if (len <= 0 || len > header_reserve) return false; - httpbuf.base = filebuf.base - len; - httpbuf.len = (size_t)len + filebuf.len; - memcpy(httpbuf.base, header, len); - header_used = len; - return true; - } - - // --- thread-safe accessors --- - int get_header_reserve() const { return header_reserve; } - int get_header_used() const { std::lock_guard lock(mutex); return header_used; } - int get_header_remaining() const { std::lock_guard lock(mutex); return header_reserve - header_used; } - bool header_fits(int len) const { return len > 0 && len <= header_reserve; } -} file_cache_ex_t; - -typedef std::shared_ptr file_cache_ex_ptr; - -class HV_EXPORT FileCacheEx : public hv::LRUCache { -public: - // --- configurable parameters (were hardcoded macros before) --- - int stat_interval; // seconds between stat() checks - int expired_time; // seconds before cache entry expires - int max_header_length; // reserved header bytes per entry - int max_file_size; // max cached file size (larger = large-file path) - - explicit FileCacheEx(size_t capacity = FILECACHE_EX_DEFAULT_MAX_NUM); - - struct OpenParam { - bool need_read; - int max_read; // per-request override for max file size - const char* path; // URL path (for directory listing) - size_t filesize; // [out] actual file size - int error; // [out] error code if Open returns NULL - - OpenParam() { - need_read = true; - max_read = FILECACHE_EX_DEFAULT_MAX_FILE_SIZE; - path = "/"; - filesize = 0; - error = 0; - } - }; - - file_cache_ex_ptr Open(const char* filepath, OpenParam* param); - bool Exists(const char* filepath) const; - bool Close(const char* filepath); - void RemoveExpiredFileCache(); - - // --- new: getters --- - int GetMaxHeaderLength() const { return max_header_length; } - int GetMaxFileSize() const { return max_file_size; } - int GetStatInterval() const { return stat_interval; } - int GetExpiredTime() const { return expired_time; } - - // --- new: setters --- - void SetMaxHeaderLength(int len) { max_header_length = len; } - void SetMaxFileSize(int size) { max_file_size = size; } - -protected: - file_cache_ex_ptr Get(const char* filepath); -}; - -#endif // HV_FILE_CACHE_EX_H_ diff --git a/http/server/HttpHandler.cpp b/http/server/HttpHandler.cpp index c618d1287..e08ad90dc 100644 --- a/http/server/HttpHandler.cpp +++ b/http/server/HttpHandler.cpp @@ -608,8 +608,8 @@ int HttpHandler::defaultStaticHandler() { return status_code; } - // FileCacheEx - FileCacheEx::OpenParam param; + // FileCache + FileCache::OpenParam param; param.max_read = service->max_file_cache_size; param.need_read = !(req->method == HTTP_HEAD || has_range); param.path = req_path; @@ -689,7 +689,7 @@ int HttpHandler::defaultErrorHandler() { std::string filepath = service->document_root + '/' + service->error_page; if (files) { // cache and load error page - FileCacheEx::OpenParam param; + FileCache::OpenParam param; fc = files->Open(filepath.c_str(), ¶m); } } @@ -798,7 +798,7 @@ int HttpHandler::GetSendData(char** data, size_t* len) { } // File service if (fc) { - // FileCacheEx + // FileCache // NOTE: no copy filebuf, more efficient header = pResp->Dump(true, false); fc->prepend_header(header.c_str(), header.size()); diff --git a/http/server/HttpHandler.h b/http/server/HttpHandler.h index 5dc0b5261..b72f07772 100644 --- a/http/server/HttpHandler.h +++ b/http/server/HttpHandler.h @@ -3,7 +3,7 @@ #include "HttpService.h" #include "HttpParser.h" -#include "FileCacheEx.h" +#include "FileCache.h" #include "WebSocketServer.h" #include "WebSocketParser.h" @@ -71,8 +71,8 @@ class HttpHandler { uint64_t last_recv_pong_time; // for sendfile - FileCacheEx *files; - file_cache_ex_ptr fc; // cache small file + FileCache *files; + file_cache_ptr fc; // cache small file struct LargeFile : public HFile { HBuf buf; uint64_t timer; diff --git a/http/server/HttpServer.cpp b/http/server/HttpServer.cpp index 9d19708e2..aa296fe8b 100644 --- a/http/server/HttpServer.cpp +++ b/http/server/HttpServer.cpp @@ -19,7 +19,7 @@ struct HttpServerPrivdata { std::vector threads; std::mutex mutex_; std::shared_ptr service; - FileCacheEx filecache; + FileCache filecache; }; static void on_recv(hio_t* io, void* buf, int readbytes) { @@ -86,7 +86,7 @@ static void on_accept(hio_t* io) { handler->service = service; // websocket service handler->ws_service = server->ws; - // FileCacheEx + // FileCache HttpServerPrivdata* privdata = (HttpServerPrivdata*)server->privdata; handler->files = &privdata->filecache; hevent_set_userdata(io, handler); @@ -132,14 +132,14 @@ static void loop_thread(void* userdata) { service->Static("/", service->document_root.c_str()); } - // FileCacheEx - FileCacheEx* filecache = &privdata->filecache; + // FileCache + FileCache* filecache = &privdata->filecache; filecache->stat_interval = service->file_cache_stat_interval; filecache->expired_time = service->file_cache_expired_time; if (filecache->expired_time > 0) { // NOTE: add timer to remove expired file cache htimer_t* timer = htimer_add(hloop, [](htimer_t* timer) { - FileCacheEx* filecache = (FileCacheEx*)hevent_userdata(timer); + FileCache* filecache = (FileCache*)hevent_userdata(timer); filecache->RemoveExpiredFileCache(); }, filecache->expired_time * 1000); hevent_set_userdata(timer, filecache); From 16c24e7dd17faad3965feb1294252b264ffbfc80 Mon Sep 17 00:00:00 2001 From: Yundi339 Date: Mon, 6 Apr 2026 00:40:24 +0800 Subject: [PATCH 08/20] refactor: enhance FileCache with safe fallback in prepend_header and add max_file_size setter --- http/server/FileCache.cpp | 126 ++++++++++++++++++++----------------- http/server/FileCache.h | 11 +++- http/server/HttpServer.cpp | 1 + 3 files changed, 76 insertions(+), 62 deletions(-) diff --git a/http/server/FileCache.cpp b/http/server/FileCache.cpp index e8bccd69b..e0c9e84a7 100644 --- a/http/server/FileCache.cpp +++ b/http/server/FileCache.cpp @@ -5,11 +5,11 @@ #include "htime.h" #include "hlog.h" -#include "httpdef.h" // import http_content_type_str_by_suffix +#include "httpdef.h" // import http_content_type_str_by_suffix #include "http_page.h" // import make_index_of_page #ifdef OS_WIN -#include "hstring.h" // import hv::utf8_to_wchar +#include "hstring.h" // import hv::utf8_to_wchar #endif #define ETAG_FMT "\"%zx-%zx\"" @@ -56,6 +56,7 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) { flags |= O_BINARY; #endif int fd = -1; + bool is_dir = false; #ifdef OS_WIN if (wfilepath.empty()) wfilepath = hv::utf8_to_wchar(filepath); if (_wstat(wfilepath.c_str(), (struct _stat*)&st) != 0) { @@ -65,7 +66,7 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) { if (S_ISREG(st.st_mode)) { fd = _wopen(wfilepath.c_str(), flags); } else if (S_ISDIR(st.st_mode)) { - fd = 0; + is_dir = true; } #else if (::stat(filepath, &st) != 0) { @@ -74,15 +75,11 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) { } fd = open(filepath, flags); #endif - if (fd < 0) { + if (fd < 0 && !is_dir) { param->error = ERR_OPEN_FILE; return NULL; } -#ifdef OS_WIN - defer(if (fd > 0) { close(fd); }) // fd=0 is Windows directory sentinel -#else - defer(close(fd);) -#endif + defer(if (fd >= 0) { close(fd); }) if (fc == NULL) { if (S_ISREG(st.st_mode) || (S_ISDIR(st.st_mode) && @@ -100,61 +97,67 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) { return NULL; } } - // Hold fc->mutex for the remainder of initialization - std::lock_guard lock(fc->mutex); - if (S_ISREG(fc->st.st_mode)) { - param->filesize = fc->st.st_size; - // FILE - if (param->need_read) { - if (fc->st.st_size > param->max_read) { - param->error = ERR_OVER_LIMIT; - // Don't cache incomplete entries - return NULL; - } - fc->resize_buf(fc->st.st_size, max_header_length); - // Loop to handle partial reads (EINTR, etc.) - char* dst = fc->filebuf.base; - size_t remaining = fc->filebuf.len; - while (remaining > 0) { - ssize_t nread = read(fd, dst, remaining); - if (nread < 0) { - if (errno == EINTR) continue; - hloge("Failed to read file: %s", filepath); - param->error = ERR_READ_FILE; + // Hold fc->mutex for initialization, but release before put() + // to avoid lock-order inversion with RemoveExpiredFileCache(). + // Lock order: LRUCache mutex → fc->mutex (never reverse). + { + std::lock_guard lock(fc->mutex); + // Sync local stat result into cached entry + fc->st = st; + if (S_ISREG(fc->st.st_mode)) { + param->filesize = fc->st.st_size; + // FILE + if (param->need_read) { + if (fc->st.st_size > param->max_read) { + param->error = ERR_OVER_LIMIT; + // Don't cache incomplete entries return NULL; } - if (nread == 0) { - hloge("Unexpected EOF reading file: %s", filepath); - param->error = ERR_READ_FILE; - return NULL; + fc->resize_buf(fc->st.st_size, max_header_length); + // Loop to handle partial reads (EINTR, etc.) + char* dst = fc->filebuf.base; + size_t remaining = fc->filebuf.len; + while (remaining > 0) { + ssize_t nread = read(fd, dst, remaining); + if (nread < 0) { + if (errno == EINTR) continue; + hloge("Failed to read file: %s", filepath); + param->error = ERR_READ_FILE; + return NULL; + } + if (nread == 0) { + hloge("Unexpected EOF reading file: %s", filepath); + param->error = ERR_READ_FILE; + return NULL; + } + dst += nread; + remaining -= nread; } - dst += nread; - remaining -= nread; } - } - const char* suffix = strrchr(filepath, '.'); - if (suffix) { - http_content_type content_type = http_content_type_enum_by_suffix(suffix + 1); - if (content_type == TEXT_HTML) { - fc->content_type = "text/html; charset=utf-8"; - } else if (content_type == TEXT_PLAIN) { - fc->content_type = "text/plain; charset=utf-8"; - } else { - fc->content_type = http_content_type_str_by_suffix(suffix + 1); + const char* suffix = strrchr(filepath, '.'); + if (suffix) { + http_content_type content_type = http_content_type_enum_by_suffix(suffix + 1); + if (content_type == TEXT_HTML) { + fc->content_type = "text/html; charset=utf-8"; + } else if (content_type == TEXT_PLAIN) { + fc->content_type = "text/plain; charset=utf-8"; + } else { + fc->content_type = http_content_type_str_by_suffix(suffix + 1); + } } + } else if (S_ISDIR(fc->st.st_mode)) { + // DIR + std::string page; + make_index_of_page(filepath, page, param->path); + fc->resize_buf(page.size(), max_header_length); + memcpy(fc->filebuf.base, page.c_str(), page.size()); + fc->content_type = "text/html; charset=utf-8"; } - } else if (S_ISDIR(fc->st.st_mode)) { - // DIR - std::string page; - make_index_of_page(filepath, page, param->path); - fc->resize_buf(page.size(), max_header_length); - memcpy(fc->filebuf.base, page.c_str(), page.size()); - fc->content_type = "text/html; charset=utf-8"; - } - gmtime_fmt(fc->st.st_mtime, fc->last_modified); - snprintf(fc->etag, sizeof(fc->etag), ETAG_FMT, - (size_t)fc->st.st_mtime, (size_t)fc->st.st_size); - // Cache the fully initialized entry + gmtime_fmt(fc->st.st_mtime, fc->last_modified); + snprintf(fc->etag, sizeof(fc->etag), ETAG_FMT, + (size_t)fc->st.st_mtime, (size_t)fc->st.st_size); + } // release fc->mutex before put() to maintain lock ordering + // Cache the fully initialized entry (acquires LRUCache mutex only) put(filepath, fc); } return fc; @@ -179,7 +182,12 @@ file_cache_ptr FileCache::Get(const char* filepath) { void FileCache::RemoveExpiredFileCache() { time_t now = time(NULL); remove_if([this, now](const std::string& filepath, const file_cache_ptr& fc) { - std::lock_guard lock(fc->mutex); + // Use try_to_lock to avoid lock-order inversion with Open(). + // If the entry is busy, skip it — it will be checked next cycle. + std::unique_lock lock(fc->mutex, std::try_to_lock); + if (!lock.owns_lock()) { + return false; + } return (now - fc->stat_time > expired_time); }); } diff --git a/http/server/FileCache.h b/http/server/FileCache.h index c43ce1943..dc9809c9a 100644 --- a/http/server/FileCache.h +++ b/http/server/FileCache.h @@ -87,9 +87,14 @@ typedef struct file_cache_s { // Thread-safe: prepend header into reserved space. // Returns true on success, false if header exceeds reserved space. + // On failure, httpbuf falls back to filebuf (body only, no header). bool prepend_header(const char* header, int len) { std::lock_guard lock(mutex); - if (len <= 0 || len > header_reserve) return false; + if (len <= 0 || len > header_reserve) { + // Safe fallback: point httpbuf at filebuf so callers always get valid data + httpbuf = filebuf; + return false; + } httpbuf.base = filebuf.base - len; httpbuf.len = (size_t)len + filebuf.len; memcpy(httpbuf.base, header, len); @@ -144,8 +149,8 @@ class HV_EXPORT FileCache : public hv::LRUCache { int GetExpiredTime() const { return expired_time; } // --- new: setters --- - void SetMaxHeaderLength(int len) { max_header_length = len; } - void SetMaxFileSize(int size) { max_file_size = size; } + void SetMaxHeaderLength(int len) { max_header_length = len < 0 ? 0 : len; } + void SetMaxFileSize(int size) { max_file_size = size < 1 ? 1 : size; } protected: file_cache_ptr Get(const char* filepath); diff --git a/http/server/HttpServer.cpp b/http/server/HttpServer.cpp index aa296fe8b..25d6d6753 100644 --- a/http/server/HttpServer.cpp +++ b/http/server/HttpServer.cpp @@ -136,6 +136,7 @@ static void loop_thread(void* userdata) { FileCache* filecache = &privdata->filecache; filecache->stat_interval = service->file_cache_stat_interval; filecache->expired_time = service->file_cache_expired_time; + filecache->max_file_size = service->max_file_cache_size; if (filecache->expired_time > 0) { // NOTE: add timer to remove expired file cache htimer_t* timer = htimer_add(hloop, [](htimer_t* timer) { From 4fd86b4a45bbb5ec671335b0c40bf94dab3ce5b2 Mon Sep 17 00:00:00 2001 From: Yundi339 Date: Mon, 6 Apr 2026 00:48:58 +0800 Subject: [PATCH 09/20] refactor: improve FileCache Open method and enhance HttpHandler header handling --- http/server/FileCache.cpp | 17 ++++++++++------- http/server/HttpHandler.cpp | 12 +++++++----- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/http/server/FileCache.cpp b/http/server/FileCache.cpp index e0c9e84a7..0cdb29072 100644 --- a/http/server/FileCache.cpp +++ b/http/server/FileCache.cpp @@ -102,17 +102,19 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) { // Lock order: LRUCache mutex → fc->mutex (never reverse). { std::lock_guard lock(fc->mutex); - // Sync local stat result into cached entry - fc->st = st; - if (S_ISREG(fc->st.st_mode)) { - param->filesize = fc->st.st_size; + if (S_ISREG(st.st_mode)) { + param->filesize = st.st_size; // FILE if (param->need_read) { - if (fc->st.st_size > param->max_read) { + if (st.st_size > param->max_read) { param->error = ERR_OVER_LIMIT; - // Don't cache incomplete entries + // Leave existing cache entry's state untouched return NULL; } + } + // Validation passed — commit new stat into cached entry + fc->st = st; + if (param->need_read) { fc->resize_buf(fc->st.st_size, max_header_length); // Loop to handle partial reads (EINTR, etc.) char* dst = fc->filebuf.base; @@ -145,8 +147,9 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) { fc->content_type = http_content_type_str_by_suffix(suffix + 1); } } - } else if (S_ISDIR(fc->st.st_mode)) { + } else if (S_ISDIR(st.st_mode)) { // DIR + fc->st = st; std::string page; make_index_of_page(filepath, page, param->path); fc->resize_buf(page.size(), max_header_length); diff --git a/http/server/HttpHandler.cpp b/http/server/HttpHandler.cpp index e08ad90dc..b5800dd7b 100644 --- a/http/server/HttpHandler.cpp +++ b/http/server/HttpHandler.cpp @@ -801,11 +801,13 @@ int HttpHandler::GetSendData(char** data, size_t* len) { // FileCache // NOTE: no copy filebuf, more efficient header = pResp->Dump(true, false); - fc->prepend_header(header.c_str(), header.size()); - *data = fc->httpbuf.base; - *len = fc->httpbuf.len; - state = SEND_DONE; - return *len; + if (fc->prepend_header(header.c_str(), header.size())) { + *data = fc->httpbuf.base; + *len = fc->httpbuf.len; + state = SEND_DONE; + return *len; + } + // Header too large for reserved space — fall through to normal path } // API service content_length = pResp->ContentLength(); From f1850b974112987f34d10680399e6afe89bb20da Mon Sep 17 00:00:00 2001 From: Yundi339 Date: Mon, 6 Apr 2026 01:12:04 +0800 Subject: [PATCH 10/20] refactor: update FileCache to reset header_used on safe fallback and use setter for max_file_size --- http/server/FileCache.h | 1 + http/server/HttpServer.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/http/server/FileCache.h b/http/server/FileCache.h index dc9809c9a..d06cf422e 100644 --- a/http/server/FileCache.h +++ b/http/server/FileCache.h @@ -93,6 +93,7 @@ typedef struct file_cache_s { if (len <= 0 || len > header_reserve) { // Safe fallback: point httpbuf at filebuf so callers always get valid data httpbuf = filebuf; + header_used = 0; return false; } httpbuf.base = filebuf.base - len; diff --git a/http/server/HttpServer.cpp b/http/server/HttpServer.cpp index 25d6d6753..6471de503 100644 --- a/http/server/HttpServer.cpp +++ b/http/server/HttpServer.cpp @@ -136,7 +136,7 @@ static void loop_thread(void* userdata) { FileCache* filecache = &privdata->filecache; filecache->stat_interval = service->file_cache_stat_interval; filecache->expired_time = service->file_cache_expired_time; - filecache->max_file_size = service->max_file_cache_size; + filecache->SetMaxFileSize(service->max_file_cache_size); if (filecache->expired_time > 0) { // NOTE: add timer to remove expired file cache htimer_t* timer = htimer_add(hloop, [](htimer_t* timer) { From 8cd32ab48b11910f3619a0b5ad8e27e0dfdbf3ad Mon Sep 17 00:00:00 2001 From: Yundi339 Date: Tue, 7 Apr 2026 13:11:39 +0800 Subject: [PATCH 11/20] refactor: update FileCache to change nread type from ssize_t to int in Open method --- cmake/vars.cmake | 1 - http/server/FileCache.cpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/cmake/vars.cmake b/cmake/vars.cmake index a425a8c75..1acf5565f 100644 --- a/cmake/vars.cmake +++ b/cmake/vars.cmake @@ -103,7 +103,6 @@ set(HTTP_SERVER_HEADERS http/server/HttpContext.h http/server/HttpResponseWriter.h http/server/WebSocketServer.h - http/server/FileCache.h ) set(MQTT_HEADERS diff --git a/http/server/FileCache.cpp b/http/server/FileCache.cpp index 0cdb29072..e2e87d0f4 100644 --- a/http/server/FileCache.cpp +++ b/http/server/FileCache.cpp @@ -120,7 +120,7 @@ file_cache_ptr FileCache::Open(const char* filepath, OpenParam* param) { char* dst = fc->filebuf.base; size_t remaining = fc->filebuf.len; while (remaining > 0) { - ssize_t nread = read(fd, dst, remaining); + int nread = read(fd, dst, remaining); if (nread < 0) { if (errno == EINTR) continue; hloge("Failed to read file: %s", filepath); From 4c1d30cc8916f9fe9d3ef40e1f6bcb890515ee9a Mon Sep 17 00:00:00 2001 From: ithewei Date: Fri, 10 Apr 2026 20:58:24 +0800 Subject: [PATCH 12/20] rm deprecated comment --- http/server/FileCache.h | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/http/server/FileCache.h b/http/server/FileCache.h index d06cf422e..697898900 100644 --- a/http/server/FileCache.h +++ b/http/server/FileCache.h @@ -4,14 +4,6 @@ /* * FileCache — Enhanced File Cache for libhv HTTP server * - * Features: - * 1. Configurable max_header_length (default 4096, tunable per-instance) - * 2. prepend_header() returns bool to report success/failure - * 3. Exposes header/buffer metrics via accessors - * 4. Fixes stat() name collision in is_modified() - * 5. max_cache_num / max_file_size configurable at runtime - * 6. Reserved header space can be tuned per-instance - * 7. Source-level API compatible; struct layout differs from original (no ABI/layout compatibility) */ #include @@ -42,7 +34,6 @@ typedef struct file_cache_s { char etag[64]; std::string content_type; - // --- new: expose header metrics --- int header_reserve; // reserved bytes before file content int header_used; // actual bytes used by prepend_header @@ -114,7 +105,6 @@ typedef std::shared_ptr file_cache_ptr; class HV_EXPORT FileCache : public hv::LRUCache { public: - // --- configurable parameters (were hardcoded macros before) --- int stat_interval; // seconds between stat() checks int expired_time; // seconds before cache entry expires int max_header_length; // reserved header bytes per entry @@ -143,13 +133,11 @@ class HV_EXPORT FileCache : public hv::LRUCache { bool Close(const char* filepath); void RemoveExpiredFileCache(); - // --- new: getters --- int GetMaxHeaderLength() const { return max_header_length; } int GetMaxFileSize() const { return max_file_size; } int GetStatInterval() const { return stat_interval; } int GetExpiredTime() const { return expired_time; } - // --- new: setters --- void SetMaxHeaderLength(int len) { max_header_length = len < 0 ? 0 : len; } void SetMaxFileSize(int size) { max_file_size = size < 1 ? 1 : size; } From 731a0a8e93da80aaaf041b70b038d25a6ee89f13 Mon Sep 17 00:00:00 2001 From: ithewei Date: Fri, 10 Apr 2026 21:17:05 +0800 Subject: [PATCH 13/20] Header too large for reserved space: send header first, then continue with file body Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- http/server/HttpHandler.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/http/server/HttpHandler.cpp b/http/server/HttpHandler.cpp index b5800dd7b..46daab398 100644 --- a/http/server/HttpHandler.cpp +++ b/http/server/HttpHandler.cpp @@ -807,7 +807,9 @@ int HttpHandler::GetSendData(char** data, size_t* len) { state = SEND_DONE; return *len; } - // Header too large for reserved space — fall through to normal path + // Header too large for reserved space: send header first, then continue with file body. + state = SEND_BODY; + goto return_header; } // API service content_length = pResp->ContentLength(); From 05248d0d8e1129e95e9723e9ccd3a361ed1252fd Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Apr 2026 12:02:02 +0800 Subject: [PATCH 14/20] fix: prevent postprocessor/errorHandler from overriding HTTP_STATUS_UNFINISHED (#828) * Initial plan * fix: prevent postprocessor/errorHandler from overriding HTTP_STATUS_UNFINISHED When a handler returns HTTP_STATUS_UNFINISHED (e.g., sendLargeFile), the postprocessor's return value was overwriting status_code, causing the connection to be treated as complete. This resulted in a premature empty response being sent and the connection being reset/closed. The fix uses local variables for the postprocessor and errorHandler return values, only checking for HTTP_STATUS_CLOSE without modifying status_code. This preserves the UNFINISHED status while still supporting the HTTP_STATUS_CLOSE feature added in commit 62cd137. Agent-Logs-Url: https://github.com/ithewei/libhv/sessions/aa478496-9e54-4c62-b0f7-810817b3867f Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com> * Refactor postprocessor variable name for clarity --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com> Co-authored-by: ithewei --- http/server/HttpHandler.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/http/server/HttpHandler.cpp b/http/server/HttpHandler.cpp index 46daab398..1b74de160 100644 --- a/http/server/HttpHandler.cpp +++ b/http/server/HttpHandler.cpp @@ -482,21 +482,25 @@ int HttpHandler::HandleHttpRequest() { } postprocessor: + // Handle HTTP_STATUS_CLOSE: close connection without response + if (status_code == HTTP_STATUS_CLOSE) { + state = WANT_CLOSE; + return HTTP_STATUS_CLOSE; + } if (status_code >= 100 && status_code < 600) { pResp->status_code = (http_status)status_code; if (pResp->status_code >= 400 && pResp->body.size() == 0 && pReq->method != HTTP_HEAD) { if (service->errorHandler) { - status_code = customHttpHandler(service->errorHandler); + int err_status_code = customHttpHandler(service->errorHandler); + if (err_status_code == HTTP_STATUS_CLOSE) { + state = WANT_CLOSE; + return HTTP_STATUS_CLOSE; + } } else { defaultErrorHandler(); } } } - // Handle HTTP_STATUS_CLOSE: close connection without response - if (status_code == HTTP_STATUS_CLOSE) { - state = WANT_CLOSE; - return HTTP_STATUS_CLOSE; - } if (fc) { pResp->content = fc->filebuf.base; pResp->content_length = fc->filebuf.len; @@ -505,8 +509,8 @@ int HttpHandler::HandleHttpRequest() { pResp->headers["Etag"] = fc->etag; } if (service->postprocessor) { - status_code = customHttpHandler(service->postprocessor); - if (status_code == HTTP_STATUS_CLOSE) { + int post_status_code = customHttpHandler(service->postprocessor); + if (post_status_code == HTTP_STATUS_CLOSE) { state = WANT_CLOSE; return HTTP_STATUS_CLOSE; } From 15809f4dda6896664ee868b1b3b6fe9527442034 Mon Sep 17 00:00:00 2001 From: Aleksey Date: Thu, 21 May 2026 12:49:57 +0300 Subject: [PATCH 15/20] fix: thread unsafe call to localtime on linux (#835) --- base/hlog.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/base/hlog.c b/base/hlog.c index 38a1eb962..24540374c 100644 --- a/base/hlog.c +++ b/base/hlog.c @@ -378,7 +378,8 @@ int logger_print(logger_t* logger, int level, const char* fmt, ...) { struct tm* tm = NULL; gettimeofday(&tv, NULL); time_t tt = tv.tv_sec; - tm = localtime(&tt); + struct tm tm_buf; + tm = localtime_r(&tt, &tm_buf); year = tm->tm_year + 1900; month = tm->tm_mon + 1; day = tm->tm_mday; From 0763810a408623fcc0c4754c1d24e17666578515 Mon Sep 17 00:00:00 2001 From: ithewei Date: Thu, 21 May 2026 18:07:58 +0800 Subject: [PATCH 16/20] Optimize http router using trie (#833) (#836) * feat: optimize http router using trie and fix wildcard match (#833) * docs: update PLAN.md * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- Makefile | 1 + docs/PLAN.md | 5 - http/server/HttpHandler.cpp | 2 +- http/server/HttpRouter.h | 224 ++++++++++++++++++++++++++++++++++ http/server/HttpService.cpp | 124 ++++++------------- http/server/HttpService.h | 33 ++--- scripts/unittest.sh | 1 + unittest/CMakeLists.txt | 5 + unittest/http_router_test.cpp | 133 ++++++++++++++++++++ 9 files changed, 421 insertions(+), 107 deletions(-) create mode 100644 http/server/HttpRouter.h create mode 100644 unittest/http_router_test.cpp diff --git a/Makefile b/Makefile index a8a43b4fb..894c530ef 100644 --- a/Makefile +++ b/Makefile @@ -281,6 +281,7 @@ unittest: prepare $(CXX) -g -Wall -O0 -std=c++11 -I. -Ibase -Icpputil -o bin/synchronized_test unittest/synchronized_test.cpp -pthread $(CXX) -g -Wall -O0 -std=c++11 -I. -Ibase -Icpputil -o bin/threadpool_test unittest/threadpool_test.cpp -pthread $(CXX) -g -Wall -O0 -std=c++11 -I. -Ibase -Icpputil -o bin/objectpool_test unittest/objectpool_test.cpp -pthread + $(CXX) -g -Wall -O0 -std=c++11 -Ihttp/server -o bin/http_router_test unittest/http_router_test.cpp $(CXX) -g -Wall -O0 -std=c++11 -I. -Ibase -Issl -Ievent -Ievpp -Icpputil -Ihttp -Ihttp/client -Ihttp/server -o bin/sizeof_test unittest/sizeof_test.cpp $(CC) -g -Wall -O0 -std=c99 -I. -Ibase -Iprotocol -o bin/nslookup unittest/nslookup_test.c protocol/dns.c base/hsocket.c base/htime.c $(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 diff --git a/docs/PLAN.md b/docs/PLAN.md index 69dac01b9..c33d5fb4b 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -9,10 +9,6 @@ - websocket client/server - mqtt client -## Improving - -- Path router: optimized matching via trie? - ## Plan - redis client @@ -21,7 +17,6 @@ - js binding - hrpc = libhv + protobuf - rudp: FEC, ARQ, UDT, QUIC -- kcptun - coroutine - cppsocket.io - IM-libhv diff --git a/http/server/HttpHandler.cpp b/http/server/HttpHandler.cpp index 1b74de160..4b413db41 100644 --- a/http/server/HttpHandler.cpp +++ b/http/server/HttpHandler.cpp @@ -284,7 +284,7 @@ void HttpHandler::onHeadersComplete() { ++p; } - if (service && service->pathHandlers.size() != 0) { + if (service && service->HasRoutes()) { service->GetRoute(pReq, &api_handler); } diff --git a/http/server/HttpRouter.h b/http/server/HttpRouter.h new file mode 100644 index 000000000..b9a955268 --- /dev/null +++ b/http/server/HttpRouter.h @@ -0,0 +1,224 @@ +#ifndef HV_HTTP_ROUTER_H_ +#define HV_HTTP_ROUTER_H_ + +#include +#include +#include +#include +#include + +namespace hv { + +template +struct RouteNode { + std::unordered_map>> children; + std::unique_ptr> param_child; + std::string param_name; + Handler handler; + bool has_handler; + + RouteNode() : has_handler(false) {} +}; + +template +struct WildcardRoute { + std::string pattern; + std::string prefix; + std::string suffix; + Handler handler; + + bool Match(const std::string& path) const { + if (!prefix.empty() && path.compare(0, prefix.size(), prefix) != 0) { + return false; + } + if (suffix.empty()) { + return true; + } + return path.size() >= prefix.size() + suffix.size() && + path.compare(path.size() - suffix.size(), suffix.size(), suffix) == 0; + } +}; + +namespace detail { + +inline std::vector splitPathSegments(const std::string& path) { + std::vector segments; + const char* p = path.c_str(); + const char* segment = NULL; + while (*p != '\0') { + if (*p == '/') { + if (segment != NULL) { + segments.push_back(std::string(segment, p - segment)); + segment = NULL; + } + } + else if (segment == NULL) { + segment = p; + } + ++p; + } + if (segment != NULL) { + segments.push_back(std::string(segment, p - segment)); + } + return segments; +} + +inline bool isParamSegment(const std::string& segment) { + // RESTful style 1 /user/:id + // RESTful style 2 /user/{id} + return (!segment.empty() && segment[0] == ':') || + (segment.size() >= 3 && segment.front() == '{' && segment.back() == '}'); +} + +inline std::string paramNameOf(const std::string& segment) { + if (!segment.empty() && segment[0] == ':') { + return segment.substr(1); + } + if (segment.size() >= 3 && segment.front() == '{' && segment.back() == '}') { + return segment.substr(1, segment.size() - 2); + } + return std::string(); +} + +template +inline bool matchNode(const RouteNode* node, const std::vector& segments, size_t index, Handler& handler, std::map& params) { + if (index == segments.size()) { + if (!node->has_handler) { + return false; + } + handler = node->handler; + return true; + } + + auto literal_iter = node->children.find(segments[index]); + if (literal_iter != node->children.end() && matchNode(literal_iter->second.get(), segments, index + 1, handler, params)) { + return true; + } + + if (node->param_child && matchNode(node->param_child.get(), segments, index + 1, handler, params)) { + params[node->param_name] = segments[index]; + return true; + } + + return false; +} + +} // namespace detail + +template +class HttpRouter { +public: + HttpRouter() : has_param_routes_(false) {} + + void Clear() { + routes_.clear(); + param_root_ = RouteNode(); + has_param_routes_ = false; + wildcard_routes_.clear(); + } + + void Insert(const std::string& path, const Handler& handler) { + // all routes + routes_[path] = handler; + + // wildcard routes + size_t wildcard_pos = path.find('*'); + if (wildcard_pos != std::string::npos) { + for (auto& route : wildcard_routes_) { + if (route.pattern == path) { + route.handler = handler; + return; + } + } + WildcardRoute route; + route.pattern = path; + route.prefix = path.substr(0, wildcard_pos); + route.suffix = path.substr(wildcard_pos + 1); + route.handler = handler; + wildcard_routes_.push_back(route); + return; + } + + // param routes + if (path.find("/:") != std::string::npos || path.find("/{") != std::string::npos) { + std::vector segments = detail::splitPathSegments(path); + RouteNode* node = ¶m_root_; + for (const auto& segment : segments) { + if (detail::isParamSegment(segment)) { + if (!node->param_child) { + node->param_child.reset(new RouteNode()); + } + node->param_name = detail::paramNameOf(segment); + node = node->param_child.get(); + continue; + } + + std::unique_ptr>& child = node->children[segment]; + if (!child) { + child.reset(new RouteNode()); + } + node = child.get(); + } + node->handler = handler; + node->has_handler = true; + has_param_routes_ = true; + } + } + + bool Find(const std::string& path, Handler& handler) const { + auto route_iter = routes_.find(path); + if (route_iter == routes_.end()) { + return false; + } + handler = route_iter->second; + return true; + } + + bool MatchParam(const std::string& path, Handler& handler, std::map& params) const { + if (!has_param_routes_) { + return false; + } + std::vector segments = detail::splitPathSegments(path); + return detail::matchNode(¶m_root_, segments, 0, handler, params); + } + + bool MatchWildcard(const std::string& path, Handler& handler) const { + for (const auto& wildcard_route : wildcard_routes_) { + if (wildcard_route.Match(path)) { + handler = wildcard_route.handler; + return true; + } + } + return false; + } + + bool Match(const std::string& path, Handler& handler, std::map& params) const { + // Literal > Param > Wildcard + return Find(path, handler) || + MatchParam(path, handler, params) || + MatchWildcard(path, handler); + } + + bool Empty() const { + return routes_.empty(); + } + + std::vector Paths() const { + std::vector paths; + paths.reserve(routes_.size()); + for (const auto& route : routes_) { + paths.push_back(route.first); + } + return paths; + } + +private: + std::unordered_map routes_; + RouteNode param_root_; + bool has_param_routes_; + std::vector> wildcard_routes_; +}; + +} + +#endif // HV_HTTP_ROUTER_H_ diff --git a/http/server/HttpService.cpp b/http/server/HttpService.cpp index acab0fa08..896113682 100644 --- a/http/server/HttpService.cpp +++ b/http/server/HttpService.cpp @@ -1,21 +1,23 @@ #include "HttpService.h" #include "HttpMiddleware.h" - -#include "hbase.h" // import hv_strendswith +#include "HttpRouter.h" namespace hv { void HttpService::AddRoute(const char* path, http_method method, const http_handler& handler) { - std::shared_ptr method_handlers = NULL; - auto iter = pathHandlers.find(path); - if (iter == pathHandlers.end()) { - // add path - method_handlers = std::make_shared(); - pathHandlers[path] = method_handlers; + if (!router) { + router = std::make_shared(); } - else { - method_handlers = iter->second; + + std::string route_path(path); + http_method_handlers_ptr method_handlers; + if (!router->Find(route_path, method_handlers)) { + // new http_method_handlers + method_handlers = std::make_shared(); + router->Insert(route_path, method_handlers); } + + // insert handler into http_method_handlers for (auto iter = method_handlers->begin(); iter != method_handlers->end(); ++iter) { if (iter->method == method) { // update @@ -27,9 +29,25 @@ void HttpService::AddRoute(const char* path, http_method method, const http_hand method_handlers->push_back(http_method_handler(method, handler)); } -int HttpService::GetRoute(const char* url, http_method method, http_handler** handler) { +bool HttpService::HasRoutes() const { + return router && !router->Empty(); +} + +hv::StringList HttpService::Paths() const { + if (!HasRoutes()) { + return hv::StringList(); + } + return router->Paths(); +} + +int HttpService::GetRoute(const char* full_path, http_method method, http_handler** handler, std::map& params) { + if (handler) *handler = NULL; + if (!HasRoutes()) { + return HTTP_STATUS_NOT_FOUND; + } + // {base_url}/path?query - const char* s = url; + const char* s = full_path; const char* b = base_url.c_str(); while (*s && *b && *s == *b) {++s;++b;} if (*b != '\0') { @@ -39,94 +57,28 @@ int HttpService::GetRoute(const char* url, http_method method, http_handler** ha while (*e && *e != '?') ++e; std::string path = std::string(s, e); - auto iter = pathHandlers.find(path); - if (iter == pathHandlers.end()) { - if (handler) *handler = NULL; + if (path.empty()) { + return HTTP_STATUS_NOT_FOUND; + } + + http_method_handlers_ptr method_handlers; + if (!router->Match(path, method_handlers, params)) { return HTTP_STATUS_NOT_FOUND; } - auto method_handlers = iter->second; for (auto iter = method_handlers->begin(); iter != method_handlers->end(); ++iter) { if (iter->method == method) { if (handler) *handler = &iter->handler; return 0; } } - if (handler) *handler = NULL; return HTTP_STATUS_METHOD_NOT_ALLOWED; } int HttpService::GetRoute(HttpRequest* req, http_handler** handler) { - // {base_url}/path?query - const char* s = req->path.c_str(); - const char* b = base_url.c_str(); - while (*s && *b && *s == *b) {++s;++b;} - if (*b != '\0') { + if (!req) { return HTTP_STATUS_NOT_FOUND; } - const char* e = s; - while (*e && *e != '?') ++e; - - std::string path = std::string(s, e); - const char *kp, *ks, *vp, *vs; - bool match; - for (auto iter = pathHandlers.begin(); iter != pathHandlers.end(); ++iter) { - kp = iter->first.c_str(); - vp = path.c_str(); - match = false; - std::map params; - - while (*kp && *vp) { - if (kp[0] == '*') { - // wildcard * - match = hv_strendswith(vp, kp+1); - break; - } else if (*kp != *vp) { - match = false; - break; - } else if (kp[0] == '/' && (kp[1] == ':' || kp[1] == '{')) { - // RESTful /:field/ - // RESTful /{field}/ - kp += 2; - ks = kp; - while (*kp && *kp != '/') {++kp;} - vp += 1; - vs = vp; - while (*vp && *vp != '/') {++vp;} - int klen = kp - ks; - if (*(ks-1) == '{' && *(kp-1) == '}') { - --klen; - } - params[std::string(ks, klen)] = std::string(vs, vp-vs); - continue; - } else { - ++kp; - ++vp; - } - } - - match = match ? match : (*kp == '\0' && *vp == '\0'); - - if (match) { - auto method_handlers = iter->second; - for (auto iter = method_handlers->begin(); iter != method_handlers->end(); ++iter) { - if (iter->method == req->method) { - for (auto& param : params) { - // RESTful /:field/ => req->query_params[field] - req->query_params[param.first] = param.second; - } - if (handler) *handler = &iter->handler; - return 0; - } - } - - if (params.size() == 0) { - if (handler) *handler = NULL; - return HTTP_STATUS_METHOD_NOT_ALLOWED; - } - } - } - if (handler) *handler = NULL; - return HTTP_STATUS_NOT_FOUND; + return GetRoute(req->path.c_str(), req->method, handler, req->query_params); } void HttpService::Static(const char* path, const char* dir) { diff --git a/http/server/HttpService.h b/http/server/HttpService.h index 16ef7a3c2..7b9686bc5 100644 --- a/http/server/HttpService.h +++ b/http/server/HttpService.h @@ -102,25 +102,31 @@ struct http_method_handler { }; // method => http_method_handler -typedef std::list http_method_handlers; +typedef std::list http_method_handlers; +typedef std::shared_ptr http_method_handlers_ptr; // path => http_method_handlers -typedef std::unordered_map> http_path_handlers; +typedef std::unordered_map http_path_handlers; namespace hv { +template +class HttpRouter; + +typedef HttpRouter http_router; + struct HV_EXPORT HttpService { /* handler chain */ // headerHandler -> preprocessor -> middleware -> processor -> postprocessor http_handler headerHandler; http_handler preprocessor; http_handlers middleware; - // processor: pathHandlers -> staticHandler -> errorHandler + // processor: router -> staticHandler -> errorHandler http_handler processor; http_handler postprocessor; /* API handlers */ - std::string base_url; - http_path_handlers pathHandlers; + std::string base_url; + std::shared_ptr router; /* Static file service */ http_handler staticHandler; @@ -183,11 +189,16 @@ struct HV_EXPORT HttpService { enable_forward_proxy = 0; } + // router interface void AddRoute(const char* path, http_method method, const http_handler& handler); + // @param[in] full_path: {base_url}/path?query + // @param[out] params: RESTful API /:field/ => params["field"] // @retval 0 OK, else HTTP_STATUS_NOT_FOUND, HTTP_STATUS_METHOD_NOT_ALLOWED - int GetRoute(const char* url, http_method method, http_handler** handler); - // RESTful API /:field/ => req->query_params["field"] + int GetRoute(const char* full_path, http_method method, http_handler** handler, std::map& params); + // @override GetRoute(req->path.c_str(), req->method, handler, req->query_params); int GetRoute(HttpRequest* req, http_handler** handler); + bool HasRoutes() const; + hv::StringList Paths() const; // Static("/", "/var/www/html") void Static(const char* path, const char* dir); @@ -209,14 +220,6 @@ struct HV_EXPORT HttpService { // @retval /api/v1/test => http://www.httpbin.org/test std::string GetProxyUrl(const char* path); - hv::StringList Paths() { - hv::StringList paths; - for (auto& pair : pathHandlers) { - paths.emplace_back(pair.first); - } - return paths; - } - // Handler = [ http_sync_handler, http_ctx_handler ] template void Use(Handler handlerFunc) { diff --git a/scripts/unittest.sh b/scripts/unittest.sh index 4d738fb69..940243443 100755 --- a/scripts/unittest.sh +++ b/scripts/unittest.sh @@ -29,3 +29,4 @@ bin/socketpair_test # bin/threadpool_test # bin/objectpool_test bin/sizeof_test +bin/http_router_test diff --git a/unittest/CMakeLists.txt b/unittest/CMakeLists.txt index 2eec34790..a2c5ff255 100644 --- a/unittest/CMakeLists.txt +++ b/unittest/CMakeLists.txt @@ -86,6 +86,10 @@ target_include_directories(ftp PRIVATE .. ../base ../protocol) add_executable(sendmail sendmail_test.c ../protocol/smtp.c ../base/hsocket.c ../base/htime.c ../util/base64.c) target_include_directories(sendmail PRIVATE .. ../base ../protocol ../util) +# ------http------ +add_executable(http_router_test http_router_test.cpp) +target_include_directories(http_router_test PRIVATE ../http/server) + if(UNIX) add_executable(webbench webbench.c) endif() @@ -115,4 +119,5 @@ add_custom_target(unittest DEPENDS ping ftp sendmail + http_router_test ) diff --git a/unittest/http_router_test.cpp b/unittest/http_router_test.cpp new file mode 100644 index 000000000..4c9fadf9e --- /dev/null +++ b/unittest/http_router_test.cpp @@ -0,0 +1,133 @@ +#include +#include + +#include +#include +#include +#include + +#include "HttpRouter.h" + +int main() { + hv::HttpRouter router; + std::map params; + int value = 0; + + router.Insert("/hello", 1); + router.Insert("/users/:id", 2); + router.Insert("/orders/{orderId}", 3); + router.Insert("/wildcard*", 4); + router.Insert("/www.*.html", 5); + router.Insert("*", 404); + router.Insert("/users/list", 6); + router.Insert("/dup", 7); + router.Insert("/dup", 8); + + assert(router.Match("/hello", value, params)); + assert(value == 1); + assert(params.empty()); + printf("Match /hello\n"); + + params.clear(); + assert(router.Match("/users/123", value, params)); + assert(value == 2); + assert(params["id"] == "123"); + printf("Match /users/:id\n"); + + params.clear(); + assert(router.Match("/orders/42", value, params)); + assert(value == 3); + assert(params["orderId"] == "42"); + printf("Match /orders/{orderId}\n"); + + params.clear(); + assert(router.Match("/wildcard-tail", value, params)); + assert(value == 4); + assert(params.empty()); + printf("Match /wildcard*\n"); + + params.clear(); + assert(router.Match("/www.index.html", value, params)); + assert(value == 5); + assert(params.empty()); + printf("Match /www.*.html\n"); + + params.clear(); + assert(router.Match("/users/list", value, params)); + assert(value == 6); + assert(params.empty()); + printf("Match /users/list\n"); + + params.clear(); + assert(router.Match("/dup", value, params)); + assert(value == 8); + assert(params.empty()); + printf("Match /dup\n"); + + params["keep"] = "yes"; + assert(router.Match("/missing", value, params)); + assert(value == 404); + assert(params["keep"] == "yes"); + printf("Match *\n"); + + std::vector paths = router.Paths(); + int dup_count = 0; + bool has_colon_param_route = false; + bool has_brace_param_route = false; + bool has_wildcard_route = false; + bool has_suffix_wildcard_route = false; + bool has_any_wildcard_route = false; + for (const auto& path : paths) { + if (path == "/dup") { + ++dup_count; + } + if (path == "/users/:id") { + has_colon_param_route = true; + } + if (path == "/orders/{orderId}") { + has_brace_param_route = true; + } + if (path == "/wildcard*") { + has_wildcard_route = true; + } + if (path == "/www.*.html") { + has_suffix_wildcard_route = true; + } + if (path == "*") { + has_any_wildcard_route = true; + } + } + assert(dup_count == 1); + assert(has_colon_param_route); + assert(has_brace_param_route); + assert(has_wildcard_route); + assert(has_suffix_wildcard_route); + assert(has_any_wildcard_route); + + hv::HttpRouter> literal_router; + std::shared_ptr literal_value; + std::map literal_params; + + literal_router.Insert("/users/:id", std::make_shared(1)); + assert(!literal_router.Find("/users/list", literal_value)); + + literal_value = std::make_shared(2); + literal_router.Insert("/users/list", literal_value); + + std::shared_ptr matched_value; + literal_params.clear(); + assert(literal_router.Match("/users/list", matched_value, literal_params)); + assert(matched_value); + assert(*matched_value == 2); + assert(literal_params.empty()); + printf("Match /users/list\n"); + + literal_params.clear(); + assert(literal_router.Match("/users/123", matched_value, literal_params)); + assert(matched_value); + assert(*matched_value == 1); + assert(literal_params["id"] == "123"); + printf("Match /users/:id\n"); + + return 0; +} From 19f3e28126bed12d594fcacf9e2ddcd377e6bc1d Mon Sep 17 00:00:00 2001 From: ithewei Date: Thu, 21 May 2026 18:21:11 +0800 Subject: [PATCH 17/20] chore: add AGENTS.md for Coding Agents (#826) --- AGENTS.md | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ CLAUDE.md | 1 + 2 files changed, 120 insertions(+) create mode 100644 AGENTS.md create mode 120000 CLAUDE.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..bc1fee447 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,119 @@ +# AGENTS.md + +This file provides guidance to agents when working with code in this repository. + +## Project Overview + +libhv is a cross-platform C/C++ network library providing event-loop with non-blocking IO and timer. Core is C99, high-level wrappers are C++11. Compatible with gcc4.8+, MSVC2015+, clang. Supports Linux, Windows, macOS, Android, iOS, BSD, Solaris. + +## Build Commands + +### Makefile (primary, Unix) +```bash +./configure --with-openssl --with-http --with-mqtt --with-kcp # configure options +make libhv # build library only (shared + static) +make # build library + examples +make examples # build all example programs +make unittest # compile unit tests +make evpp # build C++ evpp tests (requires libhv built first) +make clean # clean build artifacts +sudo make install # install to /usr/local/include/hv and /usr/local/lib +``` + +### CMake (cross-platform) +```bash +mkdir build && cd build +cmake .. -DWITH_OPENSSL=ON -DWITH_HTTP=ON -DBUILD_EXAMPLES=ON +cmake --build . +# Windows: cmake .. -G "Visual Studio 17 2022" -A x64 +``` + +### Bazel +```bash +bazel build libhv +``` + +### Package Managers +```bash +vcpkg install libhv # vcpkg +xrepo install libhv # xmake +``` + +## Testing + +```bash +make unittest # compile all unit tests +make run-unittest # compile and run unit tests (calls scripts/unittest.sh) +bash scripts/unittest.sh # run pre-built unit tests +make check # integration test: builds httpd, runs HTTP checks (scripts/check.sh) +``` + +Run a single unit test directly: +```bash +bin/rbtree_test # or any test binary in bin/ +``` + +Run evpp C++ tests (link against libhv): +```bash +make evpp +bin/TcpServer_test +bin/EventLoop_test +``` + +## Key Configuration Options + +Build flags via `./configure` or CMake `-D` options (see `config.ini` for defaults): + +| Makefile flag | CMake flag | Purpose | +|---|---|---| +| `--with-openssl` | `-DWITH_OPENSSL=ON` | SSL/TLS via OpenSSL | +| `--with-gnutls` | `-DWITH_GNUTLS=ON` | SSL/TLS via GnuTLS | +| `--with-mbedtls` | `-DWITH_MBEDTLS=ON` | SSL/TLS via mbedTLS | +| `--with-nghttp2` | `-DWITH_NGHTTP2=ON` | HTTP/2 support | +| `--with-kcp` | `-DWITH_KCP=ON` | KCP reliable UDP | +| `--with-mqtt` | `-DWITH_MQTT=ON` | MQTT client | +| `--with-protocol` | `-DWITH_PROTOCOL=ON` | ICMP, DNS, FTP, SMTP | +| `--with-evpp` | `-DWITH_EVPP=ON` | C++ wrappers (default: yes) | +| `--without-evpp` | `-DWITH_EVPP=OFF` | Pure C build, no C++ | +| `--enable-uds` | `-DENABLE_UDS=ON` | Unix Domain Socket | +| `--with-io-uring` | `-DWITH_IO_URING=ON` | io_uring event backend (Linux 5.1+) | + +## Architecture + +``` +Application / Examples + │ + ├── http/server, http/client HTTP/WebSocket/gRPC (C++) + ├── mqtt/ MQTT client (C) + ├── protocol/ ICMP, DNS, FTP, SMTP (C) + │ + ├── evpp/ C++ wrappers: TcpServer, TcpClient, UdpServer, EventLoop + │ + ├── event/ Core event loop: hloop, hio, htimer + │ └── backends: epoll (Linux), kqueue (macOS/BSD), iocp/wepoll (Windows), io_uring (Linux 5.1+), select (fallback) + │ └── kcp/ KCP reliable UDP transport + │ + ├── ssl/ Unified SSL interface (OpenSSL / GnuTLS / mbedTLS / platform) + │ + ├── base/ Platform abstraction, sockets, threads, logging, data structures + ├── util/ C utilities (base64, md5, sha1) + └── cpputil/ C++ utilities (string, path, file, json, threadpool, ini parser) +``` + +**Layering rules**: `base/` has no dependencies on other modules. `event/` depends on `base/` and `ssl/`. `evpp/` wraps `event/`. `http/` depends on `evpp/`. Higher layers are optional and controlled by build flags. + +**Public API entry point**: `hv.h` includes all base headers. Module-specific headers (e.g., `HttpServer.h`, `TcpServer.h`, `mqtt_client.h`) are the primary include for each feature. Installed headers go to `include/hv/`. + +**Key types**: `hloop_t` (event loop), `hio_t` (IO handle), `htimer_t` (timer) in the C API. `EventLoop`, `TcpServer`, `TcpClient`, `Channel` in the C++ API. `HttpRequest`, `HttpResponse`, `HttpService` for HTTP. + +## Code Style + +- **Formatting**: `.clang-format` — LLVM-based, 4-space indent, 160 column limit, pointer right-aligned, `catch`/`else` on new line (custom brace wrapping), no include sorting. +- **C API naming**: `h` prefix for functions (`hloop_new`, `hio_read`), `_t` suffix for types (`hloop_t`, `hio_t`), UPPERCASE macros (`HV_EXPORT`). +- **C++ API naming**: PascalCase classes (`EventLoop`, `TcpServer`), `hv` namespace. +- **File naming**: lowercase with underscores (`.c` for C, `.cpp` for C++). +- **Platform-specific code**: isolated via `hplatform.h` and `#ifdef` conditional compilation. + +## CI + +GitHub Actions (`.github/workflows/CI.yml`): builds and tests on Linux (with OpenSSL+nghttp2+KCP+MQTT), Windows (CMake + VS2022), macOS, Android (NDK cross-compile), iOS (Xcode cross-compile). Benchmark workflow runs echo-server throughput and HTTP performance comparisons. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 120000 index 000000000..47dc3e3d8 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file From 9c135991cf6b3af8f2ac1a14b274b7a73985fe4a Mon Sep 17 00:00:00 2001 From: ithewei Date: Sat, 23 May 2026 13:39:19 +0800 Subject: [PATCH 18/20] Apply suggestions from ai code review (#837) * fix: apply suggestions for base dir from ai code review * fix: apply suggestions for cpputil dir from ai code review * fix: apply suggestions for ssl dir from ai code review * fix: apply suggestions for event dir from ai code review --- base/hbase.c | 23 ++++- base/hbase.h | 2 +- base/hdef.h | 2 +- base/hlog.c | 75 +++++++++----- base/hmath.h | 2 +- base/htime.c | 36 ++++--- base/htime.h | 18 ++++ base/queue.h | 13 ++- 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 ++- event/epoll.c | 21 +++- event/evport.c | 37 ++++--- 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 +++- ssl/gnutls.c | 41 ++++++-- ssl/mbedtls.c | 22 ++++- ssl/openssl.c | 7 +- 33 files changed, 522 insertions(+), 243 deletions(-) diff --git a/base/hbase.c b/base/hbase.c index 583161271..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; @@ -501,8 +502,20 @@ 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') { + ret = -2; + break; + } + parsed_port = parsed_port * 10 + (port[i] - '0'); + if (parsed_port > 65535) { + ret = -3; + break; + } + } + if (ret == 0) { + stURL->port = (unsigned short)parsed_port; } } else { port = ep; @@ -518,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/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..092fe6ae0 100644 --- a/base/hlog.c +++ b/base/hlog.c @@ -29,9 +29,44 @@ //#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, 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) { + s_gmtoff -= SECONDS_PER_DAY; + } +} struct logger_s { logger_handler handler; @@ -79,13 +114,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); @@ -214,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) { @@ -375,17 +406,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++;\ 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; \ } \ } diff --git a/event/epoll.c b/event/epoll.c index 5fc6871e9..b7b28214b 100644 --- a/event/epoll.c +++ b/event/epoll.c @@ -4,6 +4,8 @@ #include "hplatform.h" #include "hdef.h" #include "hevent.h" +#include "hlog.h" +#include "hsocket.h" #ifdef OS_WIN #include "wepoll/wepoll.h" @@ -25,9 +27,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 +51,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 +76,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 +111,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..7ddff7510 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,27 @@ 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) { + if (errno == EINTR) { + return 0; + } + perror("port_getn"); + 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..ffd939653 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; 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 33ce9fc4d6d71504f9074ccd05063381e9e44dd3 Mon Sep 17 00:00:00 2001 From: ithewei Date: Sat, 23 May 2026 17:02:55 +0800 Subject: [PATCH 19/20] Apply suggestions for evpp dir from ai code review (#838) --- 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 51089c65c6ced0f78f401566407acf6cc4cad977 Mon Sep 17 00:00:00 2001 From: ithewei Date: Sat, 23 May 2026 23:42:42 +0800 Subject: [PATCH 20/20] Apply suggestions for http dir from ai code review (#839) --- 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 4b413db41..9362806ae 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; @@ -1050,9 +1055,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() { @@ -1083,8 +1087,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); @@ -1106,10 +1109,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");