From 153ba77107f0be57cb7f29852fff58d7fc13c185 Mon Sep 17 00:00:00 2001
From: Kevin Tang <zhikait@codeaurora.org>
Date: Mon, 20 Jul 2015 15:36:06 -0700
Subject: [PATCH] crash in LocTimer wrapper

LocTimerWrapper implements loc_timer_start/stop calls for backward
compatibility. There is a race condtiion where the wrapper object
could be deleted by the client thread and expireation handling at
rough the same time, which would have the memory freed twice. Now
they are mutext protected.

Change-Id: I25d7466db88a840a8a09e7a476cface48c91d22e
---
 utils/LocTimer.cpp | 26 +++++++++++++++++++++-----
 utils/loc_timer.h  |  6 ++++--
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/utils/LocTimer.cpp b/utils/LocTimer.cpp
index e1be1e8a..277a8135 100644
--- a/utils/LocTimer.cpp
+++ b/utils/LocTimer.cpp
@@ -539,22 +539,38 @@ bool LocTimer::stop() {
 class LocTimerWrapper : public LocTimer {
     loc_timer_callback mCb;
     void* mCallerData;
+    static pthread_mutex_t mMutex;
+    inline ~LocTimerWrapper() { mCb = NULL; }
 public:
     inline LocTimerWrapper(loc_timer_callback cb, void* callerData) :
         mCb(cb), mCallerData(callerData) {}
+    inline bool isAlive() { return mCb != NULL; }
+    void destroy() {
+        pthread_mutex_lock(&mMutex);
+        if (isAlive()) {
+            delete this;
+        }
+        pthread_mutex_unlock(&mMutex);
+    }
     inline virtual void timeOutCallback() {
         mCb(mCallerData, 0);
-        delete this;
+        destroy();
     }
 };
 
+pthread_mutex_t LocTimerWrapper::mMutex = PTHREAD_MUTEX_INITIALIZER;
+
 void* loc_timer_start(uint64_t msec, loc_timer_callback cb_func,
                       void *caller_data, bool wake_on_expire)
 {
-    LocTimerWrapper* locTimerWrapper = new LocTimerWrapper(cb_func, caller_data);
+    LocTimerWrapper* locTimerWrapper = NULL;
 
-    if (locTimerWrapper) {
-        locTimerWrapper->start(msec, wake_on_expire);
+    if (cb_func) {
+        locTimerWrapper = new LocTimerWrapper(cb_func, caller_data);
+
+        if (locTimerWrapper) {
+            locTimerWrapper->start(msec, wake_on_expire);
+        }
     }
 
     return locTimerWrapper;
@@ -565,7 +581,7 @@ void loc_timer_stop(void*&  handle)
     if (handle) {
         LocTimerWrapper* locTimerWrapper = (LocTimerWrapper*)(handle);
         locTimerWrapper->stop();
-        delete locTimerWrapper;
+        locTimerWrapper->destroy();
         handle = NULL;
     }
 }
diff --git a/utils/loc_timer.h b/utils/loc_timer.h
index 8836d1ea..2967858e 100644
--- a/utils/loc_timer.h
+++ b/utils/loc_timer.h
@@ -45,7 +45,8 @@ typedef void (*loc_timer_callback)(void *user_data, int32_t result);
 
 /*
     delay_msec:         timeout value for the timer.
-    loc_timer_callback: callback function pointer, implemented by client.
+    cb_func:            callback function pointer, implemented by client.
+                        Can not be NULL.
     user_data:          client context pointer, passthrough.  Will be
                         returned when loc_timer_callback() is called.
     wakeOnExpire:       true if to wake up CPU (if sleeping) upon timer
@@ -53,9 +54,10 @@ typedef void (*loc_timer_callback)(void *user_data, int32_t result);
                         false if to wait until next time CPU wakes up (if
                                  sleeping) and then notify the client.
     Returns the handle, which can be used to stop the timer
+                        NULL, if timer start fails (e.g. if cb_func is NULL).
 */
 void* loc_timer_start(uint64_t delay_msec,
-                      loc_timer_callback,
+                      loc_timer_callback cb_func,
                       void *user_data,
                       bool wake_on_expire=false);