Ubshm transport dev - #9
Open
zchuango wants to merge 31 commits into
Open
Conversation
|
你好，请问你是HW的吗？是否可以认识下 |
Co-authored-by: 郭业昌 <lvpengfei@MacBook-Air.local>
* 修复ubring server端关闭连接coredump问题 * 修复PollIn/PollOut解引用已释放Socket指针的问题 PollIn/PollOut通过ep->_socket(裸指针)读取data socket,当data socket 被销毁时该指针悬空,导致Socket::Address读到垃圾id触发SIGSEGV。 改为存储_socket_id(SocketId),用Address获取引用计数的Socket, 并在整个回调期间持有该引用,避免解引用悬空指针。 * 修复client非正常退出导致UBRING shm残留的问题 client被强杀(SIGTERM/崩溃/OOM)时teardown没跑完,localShm(_C)的 shm_unlink未执行,导致/dev/shm残留_C文件。server的remoteShm只munmap 不unlink(正确),无法清理client的名字。 在握手ESTABLISHED时(client/server都确认对方已mmap自己的localShm) 立即unlink localShm名字。此时对端已持有mmap引用,unlink只删名字不 影响通信;进程任意时刻退出都不会残留文件名。 * Address chenBright's review: use English comments and BAIDU_CACHELINE_ALIGNMENT - Convert all Chinese comments in ubshm to English (per chenBright's 'Please use English' on ub_endpoint.cpp:723, ub_ring.cpp:337, shm_ubs.cpp:316, and similar) - Replace __attribute__((aligned(64))) with BAIDU_CACHELINE_ALIGNMENT in ubr_msg.h (per chenBright's comment on ubr_msg.h:41) - Remove unnecessary TODO comment in ub_ring.cpp:551 (per chenBright's 'Unnecessary comments, please delete') * Remove unused lock macros in thread_lock.h Per chenBright's review, the functions and macros defined in thread_lock.h are largely unused. Verified usage across ubshm: - LOCK_GUARD / UnlockMutex: 8 call sites in shm_ubs.cpp and ub_ring_manager.cpp, kept. - SPIN_LOCK_GUARD, R_LOCK_GUARD, W_LOCK_GUARD, SEMAPHORE_WAIT_GUARD, SEMAPHORE_WAIT_GUARD_WITH_CLOSE and their helper functions (UnlockSpinLock, UnlockRWLock, PostSem, PostSemWithClose): 0 call sites, removed. * Apply chenBright's review on timer_mgr globals Per chenBright's review on timer_mgr.cpp:32-37: - Add explicit default values to uninitialized globals (g_total_timer_num=0, g_max_system_fd=0, g_epoll_execute_thread=0, g_timer_module_initialized=0) - Rename globals to snake_case (g_epollFd -> g_epoll_fd, g_totalTimerNum -> g_total_timer_num, g_timerFdCtxMap -> g_timer_fd_ctx_map, maxSystemFd -> g_max_system_fd, g_epollExecuteThread -> g_epoll_execute_thread, g_timerModuleInitialized -> g_timer_module_initialized) - maxSystemFd also gains the g_ prefix to match global naming style Also fix the missing std:: qualifier on atomic_fetch_sub/add/load (per chenBright's earlier comment on timer_mgr.cpp:80). * Change CloseTimerFd fd type from uint32_t to int Per chenBright's review on timer_mgr.cpp:399 (uint32_t -> int). fd is a system file descriptor; POSIX APIs use int and -1 denotes an invalid fd, which uint32_t cannot represent. Changed the CloseTimerFd signature (header + definition) and removed the now-unnecessary (uint32_t) casts at the two call sites. * Use BAIDU_LIKELY/BAIDU_UNLIKELY instead of custom __builtin_expect Per chenBright's review on common.h:27. Rather than redefine the macros with __builtin_expect directly, forward LIKELY/UNLIKELY to brpc's standard BAIDU_LIKELY/BAIDU_UNLIKELY (from butil/compiler_specific.h). The 122 call sites keep using LIKELY()/UNLIKELY() unchanged; only the macro bodies change, preserving semantics. * Add unit tests for UBShmEndpoint Per chenBright's request to add unit tests for UBShmTransport in this PR (rather than a follow-up). Adds test/brpc_ubring_unittest.cpp with tests covering the public interface of UBShmEndpoint under the g_skip_ub_init=true mode (which skips real shared-memory/poller setup): - construct_and_destruct: lifecycle safety - is_writable_false_when_skip_init: skip-mode behavior - reset_is_idempotent: Reset() is safe to call repeatedly The file follows the brpc_*_unittest.cpp naming convention so it is auto-collected by test/CMakeLists.txt's file(GLOB). Verified: compiles, links, and all 3 tests pass (g++ 15.2, C++17, gtest, BRPC_WITH_UBRING=ON). * Rewrite UBShmEndpoint unit tests with real coverage Per chenBright's feedback that the previous tests were too simple and did not cover the main methods. Source changes to enable testing: - Move HelloMessage struct declaration from ub_endpoint.cpp to ub_endpoint.h so tests can access it - Expose private members under #ifdef UNIT_TEST (precedent: butil/containers/stack_container.h) so tests can call AllocateClientResources without -Dprivate=public (which breaks GCC 15 + new libstdc++ <any>/<sstream>) Tests (9, all passing on Ubuntu 26.04 g++ 15.2 C++17 gtest): HelloMessageTest (5): serialize/deserialize roundtrip, network byte order verification, uint64 max boundary, full shm_name, toString UBShmEndpointTest (4): construct, real IPC shm AllocateClientResources (g_skip_ub_init=false), reset cleanup, reset idempotency
# Conflicts: # CMakeLists.txt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: resolve
Problem Summary:
What is changed and the side effects?
Changed:
Side effects:
Performance effects:
Breaking backward compatibility:
Check List: