ソースを参照

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 <herman.chen@rock-chips.com>
Change-Id: I81ead0f796ba6e26b520a87ae69cc8f7f6e816f4
Herman Chen 8 ヶ月 前
コミット
8108bf58ea
2 ファイル変更60 行追加40 行削除
  1. 6 6
      osal/inc/mpp_thread.h
  2. 54 34
      osal/mpp_mem.cpp

+ 6 - 6
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:

+ 54 - 34
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();
 }