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) #1 pc 000000000005363c /apex/com.android.runtime/lib64/bionic/libc.so (__fortify_fatal(char const*, ...)+124) (BuildId: 4e07915368c859b1910c68c84a8de75f) #2 pc 00000000000b74cc /apex/com.android.runtime/lib64/bionic/libc.so (HandleUsingDestroyedMutex(pthread_mutex_t*, char const*)+60) (BuildId: 4e07915368c859b1910c68c84a8de75f) #3 pc 00000000000b735c /apex/com.android.runtime/lib64/bionic/libc.so (pthread_mutex_lock+240) (BuildId: 4e07915368c859b1910c68c84a8de75f) #4 pc 0000000000048290 /system/bin/mpp_trie_test (mpp_osal_free+108) (BuildId: 55dca41ecc701b3ad16f0ef02270a45ce40533ff) #5 pc 0000000000041080 /system/bin/mpp_trie_test (MppMemPoolService::~MppMemPoolService()+32) (BuildId: 55dca41ecc701b3ad16f0ef02270a45ce40533ff) #6 pc 00000000000b9ca4 /apex/com.android.runtime/lib64/bionic/libc.so (__cxa_finalize+280) (BuildId: 4e07915368c859b1910c68c84a8de75f) #7 pc 00000000000ac944 /apex/com.android.runtime/lib64/bionic/libc.so (exit+24) (BuildId: 4e07915368c859b1910c68c84a8de75f) #8 pc 000000000004a1f8 /apex/com.android.runtime/lib64/bionic/libc.so (__libc_init+100) (BuildId: 4e07915368c859b1910c68c84a8de75f) Signed-off-by: Herman Chen <[email protected]> Change-Id: I81ead0f796ba6e26b520a87ae69cc8f7f6e816f4
rockchip-linux · Aug 16, 2024 · 8108bf5 · 8108bf5
1 parent 083b769
commit 8108bf5
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 40 deletions.
diff --git a/osal/inc/mpp_thread.h b/osal/inc/mpp_thread.h
@@ -89,17 +89,17 @@ class Mutex
     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
@@ -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 @@ class MppMemService
     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 @@ class MppMemService
     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,16 +659,16 @@ 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) {
                 ptr = (RK_U8 *)ptr + MEM_ALIGN;
                 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();
 }