From 8108bf58ea53dfe1bf3069c1a4f22735d7128cda Mon Sep 17 00:00:00 2001 From: Herman Chen Date: Fri, 16 Aug 2024 09:24:55 +0800 Subject: [PATCH] fix[osal]: Fix mpp_mem single instance issue When another C++ static global object init before the mpp_mem service the MppService service will be inited twice. Then on object destroy will deinit service twice and cause mutex double delete issue. On init E mpp_mem : MppMemService start 0 0x7c536619e8 I mpp_mem : MppMemService mpp_mem_debug enabled 3 max node 1024 E mpp_mem : MppMemService start 1 0x5e8d724230 I mpp_mem : MppMemService mpp_mem_debug enabled 3 max node 1024 On destory 05-17 09:58:04.743 2576 2576 E mpp_mem : ~MppMemService enter 0 0x5e8d724230 05-17 09:58:04.743 2576 2576 E mpp_mem : ~MppMemService enter 1 0x7c536619e8 05-17 09:58:04.743 2576 2576 E mpp_mem : mpp_osal_free *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** Build fingerprint: 'rockchip/rk3576_t/rk3576_t:13/TQ3C.230805.001.B2/eng.kenjc.20240510.161710:userdebug/release-keys' Revision: '0' ABI: 'arm64' Timestamp: 2024-05-17 09:58:04.800905936+0000 Process uptime: 1s Cmdline: mpp_trie_test pid: 2576, tid: 2576, name: mpp_trie_test >>> mpp_trie_test <<< uid: 0 tagged_addr_ctrl: 0000000000000001 (PR_TAGGED_ADDR_ENABLE) signal 6 (SIGABRT), code -1 (SI_QUEUE), fault addr -------- Abort message: 'FORTIFY: pthread_mutex_lock called on a destroyed mutex (0x5e8d724230)' x0 0000000000000000 x1 0000000000000a10 x2 0000000000000006 x3 0000007fd26f05d0 x4 0000000000008080 x5 0000000000008080 x6 0000000000008080 x7 8080000000000000 x8 00000000000000f0 x9 0000007c50d22a00 x10 0000000000000001 x11 0000007c50d60de4 x12 0101010101010101 x13 000000007fffffff x14 000000000001ffea x15 0000000000000078 x16 0000007c50dc5d58 x17 0000007c50da2c70 x18 0000007c55b38000 x19 0000000000000a10 x20 0000000000000a10 x21 00000000ffffffff x22 0000000000001000 x23 0000005e8d724230 x24 0000007c5489e010 x25 0000005e8d70c060 x26 0000000000000002 x27 0000007c513226e8 x28 0000000000000000 x29 0000007fd26f0650 lr 0000007c50d52968 sp 0000007fd26f05b0 pc 0000007c50d52994 pst 0000000000000000 backtrace: #00 pc 0000000000051994 /apex/com.android.runtime/lib64/bionic/libc.so (abort+164) (BuildId: 4e07915368c859b1910c68c84a8de75f) #01 pc 000000000005363c /apex/com.android.runtime/lib64/bionic/libc.so (__fortify_fatal(char const*, ...)+124) (BuildId: 4e07915368c859b1910c68c84a8de75f) #02 pc 00000000000b74cc /apex/com.android.runtime/lib64/bionic/libc.so (HandleUsingDestroyedMutex(pthread_mutex_t*, char const*)+60) (BuildId: 4e07915368c859b1910c68c84a8de75f) #03 pc 00000000000b735c /apex/com.android.runtime/lib64/bionic/libc.so (pthread_mutex_lock+240) (BuildId: 4e07915368c859b1910c68c84a8de75f) #04 pc 0000000000048290 /system/bin/mpp_trie_test (mpp_osal_free+108) (BuildId: 55dca41ecc701b3ad16f0ef02270a45ce40533ff) #05 pc 0000000000041080 /system/bin/mpp_trie_test (MppMemPoolService::~MppMemPoolService()+32) (BuildId: 55dca41ecc701b3ad16f0ef02270a45ce40533ff) #06 pc 00000000000b9ca4 /apex/com.android.runtime/lib64/bionic/libc.so (__cxa_finalize+280) (BuildId: 4e07915368c859b1910c68c84a8de75f) #07 pc 00000000000ac944 /apex/com.android.runtime/lib64/bionic/libc.so (exit+24) (BuildId: 4e07915368c859b1910c68c84a8de75f) #08 pc 000000000004a1f8 /apex/com.android.runtime/lib64/bionic/libc.so (__libc_init+100) (BuildId: 4e07915368c859b1910c68c84a8de75f) Signed-off-by: Herman Chen Change-Id: I81ead0f796ba6e26b520a87ae69cc8f7f6e816f4 --- osal/inc/mpp_thread.h | 12 +++--- osal/mpp_mem.cpp | 88 ++++++++++++++++++++++++++----------------- 2 files changed, 60 insertions(+), 40 deletions(-) diff --git a/osal/inc/mpp_thread.h b/osal/inc/mpp_thread.h index 19688cb3..3c3550ce 100644 --- a/osal/inc/mpp_thread.h +++ b/osal/inc/mpp_thread.h @@ -89,17 +89,17 @@ public: public: inline Autolock(Mutex* mutex, RK_U32 enable = 1) : mEnabled(enable), - mLock(*mutex) { - if (mEnabled) - mLock.lock(); + mLock(mutex) { + if (mLock && mEnabled) + mLock->lock(); } inline ~Autolock() { - if (mEnabled) - mLock.unlock(); + if (mLock && mEnabled) + mLock->unlock(); } private: RK_S32 mEnabled; - Mutex& mLock; + Mutex *mLock; }; private: diff --git a/osal/mpp_mem.cpp b/osal/mpp_mem.cpp index 6c5f0c27..23a4cdcd 100644 --- a/osal/mpp_mem.cpp +++ b/osal/mpp_mem.cpp @@ -51,7 +51,7 @@ do { \ if (!(cond)) { \ mpp_err("found mpp_mem assert failed, start dumping:\n"); \ - service.dump(__FUNCTION__); \ + MppMemService::get_inst()->dump(__FUNCTION__); \ mpp_assert(cond); \ } \ } while (0) @@ -93,9 +93,10 @@ typedef struct MppMemLog_s { class MppMemService { public: - // avoid any unwanted function - MppMemService(); - ~MppMemService(); + static MppMemService *get_inst() { + static MppMemService instance; + return &instance; + } void add_node(const char *caller, void *ptr, size_t size); /* @@ -121,10 +122,16 @@ public: RK_U32 total_now(void) { return m_total_size; } RK_U32 total_max(void) { return m_total_max; } - Mutex lock; + Mutex *m_lock; RK_U32 debug; private: + // avoid any unwanted function + MppMemService(); + ~MppMemService(); + MppMemService(const MppMemService &); + MppMemService &operator=(const MppMemService &); + // data for node record and delay free check RK_S32 nodes_max; RK_S32 nodes_idx; @@ -145,13 +152,8 @@ private: MppMemLog *logs; RK_U32 m_total_size; RK_U32 m_total_max; - - MppMemService(const MppMemService &); - MppMemService &operator=(const MppMemService &); }; -static MppMemService service; - static const char *ops2str[MEM_OPS_BUTT] = { "malloc", "realloc", @@ -213,6 +215,8 @@ MppMemService::MppMemService() { mpp_env_get_u32("mpp_mem_debug", &debug, 0); + m_lock = new Mutex(); + // add more flag if debug enabled if (debug) debug |= MEM_DEBUG_EN; @@ -247,7 +251,7 @@ MppMemService::MppMemService() MppMemService::~MppMemService() { if (debug & MEM_DEBUG_EN) { - AutoMutex auto_lock(&lock); + AutoMutex auto_lock(m_lock); RK_S32 i = 0; MppMemNode *node = nodes; @@ -296,6 +300,11 @@ MppMemService::~MppMemService() os_free(frees); os_free(logs); } + + if (m_lock) { + delete m_lock; + m_lock = NULL; + } } void MppMemService::add_node(const char *caller, void *ptr, size_t size) @@ -561,9 +570,10 @@ void MppMemService::reset_node(const char *caller, void *ptr, void *ret, size_t void MppMemService::add_log(MppMemOps ops, const char *caller, void *ptr, void *ret, size_t size_0, size_t size_1) { + MppMemService *srv = MppMemService::get_inst(); MppMemLog *log = &logs[log_idx]; - if (service.debug & MEM_RUNTIME_LOG) + if (srv->debug & MEM_RUNTIME_LOG) mpp_log("%-7s ptr %010p %010p size %8u %8u at %s\n", ops2str[ops], ptr, ret, size_0, size_1, caller); @@ -639,7 +649,8 @@ void MppMemService::dump(const char *caller) void *mpp_osal_malloc(const char *caller, size_t size) { - RK_U32 debug = service.debug; + MppMemService *srv = MppMemService::get_inst(); + RK_U32 debug = srv->debug; size_t size_align = MEM_ALIGNED(size); size_t size_real = (debug & MEM_EXT_ROOM) ? (size_align + 2 * MEM_ALIGN) : (size_align); @@ -648,8 +659,8 @@ void *mpp_osal_malloc(const char *caller, size_t size) os_malloc(&ptr, MEM_ALIGN, size_real); if (debug) { - AutoMutex auto_lock(&service.lock); - service.add_log(MEM_MALLOC, caller, NULL, ptr, size, size_real); + AutoMutex auto_lock(srv->m_lock); + srv->add_log(MEM_MALLOC, caller, NULL, ptr, size, size_real); if (ptr) { if (debug & MEM_EXT_ROOM) { @@ -657,7 +668,7 @@ void *mpp_osal_malloc(const char *caller, size_t size) set_mem_ext_room(ptr, size); } - service.add_node(caller, ptr, size); + srv->add_node(caller, ptr, size); } } @@ -674,7 +685,8 @@ void *mpp_osal_calloc(const char *caller, size_t size) void *mpp_osal_realloc(const char *caller, void *ptr, size_t size) { - RK_U32 debug = service.debug; + MppMemService *srv = MppMemService::get_inst(); + RK_U32 debug = srv->debug; void *ret; if (NULL == ptr) @@ -696,15 +708,15 @@ void *mpp_osal_realloc(const char *caller, void *ptr, size_t size) // if realloc fail the original buffer will be kept the same. mpp_err("mpp_realloc ptr %p to size %d failed\n", ptr, size); } else { - AutoMutex auto_lock(&service.lock); + AutoMutex auto_lock(srv->m_lock); // if realloc success reset the node and record if (debug) { void *ret_ptr = (debug & MEM_EXT_ROOM) ? ((RK_U8 *)ret + MEM_ALIGN) : (ret); - service.reset_node(caller, ptr, ret_ptr, size); - service.add_log(MEM_REALLOC, caller, ptr, ret_ptr, size, size_real); + srv->reset_node(caller, ptr, ret_ptr, size); + srv->add_log(MEM_REALLOC, caller, ptr, ret_ptr, size, size_real); ret = ret_ptr; } } @@ -714,7 +726,9 @@ void *mpp_osal_realloc(const char *caller, void *ptr, size_t size) void mpp_osal_free(const char *caller, void *ptr) { - RK_U32 debug = service.debug; + MppMemService *srv = MppMemService::get_inst(); + RK_U32 debug = srv->debug; + if (NULL == ptr) return; @@ -725,40 +739,46 @@ void mpp_osal_free(const char *caller, void *ptr) size_t size = 0; - AutoMutex auto_lock(&service.lock); + AutoMutex auto_lock(srv->m_lock); if (debug & MEM_POISON) { // NODE: keep this node and delete delay node - void *ret = service.delay_del_node(caller, ptr, &size); + void *ret = srv->delay_del_node(caller, ptr, &size); if (ret) os_free((RK_U8 *)ret - MEM_ALIGN); - service.add_log(MEM_FREE_DELAY, caller, ptr, ret, size, 0); + srv->add_log(MEM_FREE_DELAY, caller, ptr, ret, size, 0); } else { void *ptr_real = (RK_U8 *)ptr - MEM_HEAD_ROOM(debug); // NODE: delete node and return size here - service.del_node(caller, ptr, &size); - service.chk_mem(caller, ptr, size); + srv->del_node(caller, ptr, &size); + srv->chk_mem(caller, ptr, size); os_free(ptr_real); - service.add_log(MEM_FREE, caller, ptr, ptr_real, size, 0); + srv->add_log(MEM_FREE, caller, ptr, ptr_real, size, 0); } } /* dump memory status */ void mpp_show_mem_status() { - AutoMutex auto_lock(&service.lock); - if (service.debug & MEM_DEBUG_EN) - service.dump(__FUNCTION__); + MppMemService *srv = MppMemService::get_inst(); + AutoMutex auto_lock(srv->m_lock); + + if (srv->debug & MEM_DEBUG_EN) + srv->dump(__FUNCTION__); } RK_U32 mpp_mem_total_now() { - AutoMutex auto_lock(&service.lock); - return service.total_now(); + MppMemService *srv = MppMemService::get_inst(); + AutoMutex auto_lock(srv->m_lock); + + return srv->total_now(); } RK_U32 mpp_mem_total_max() { - AutoMutex auto_lock(&service.lock); - return service.total_max(); + MppMemService *srv = MppMemService::get_inst(); + AutoMutex auto_lock(srv->m_lock); + + return srv->total_max(); }