commit b117c7f9892ae0bddd11bc6f2aaa18729fbcd429
author Martin Brabham <optedoblivion@google.com> Thu Apr 04 14:57:41 2019 -0700
committer android-build-team Robot <android-build-team-robot@google.com> Tue May 14 04:58:02 2019 +0000
tree 29aed4828f5edc5fb840fd562075eb8b3b25712e
parent d0b9f6155d878426fa948b28b51b4b4c5989201e

DO NOT MERGE: osi: Offload mutex pointer to local scope

Create a shared_ptr for the callback_mutex in the alarm struct.
When performing the callback, make a local shared_ptr reference.
lock_guard on the local shared_ptr reference.

Bug: 117997080
Test: atest net_test_bluetooth
Change-Id: Iab800f720f4ccc4735e4d494e0d458eb97b40a4a
(cherry picked from commit 947c58718f93629f2fba6e16d5163b6da07d0056)

osi/src/alarm.cc

diff --git a/osi/src/alarm.cc b/osi/src/alarm.cc
index 83a661d..07d8704 100644
--- a/osi/src/alarm.cc
+++ b/osi/src/alarm.cc
@@ -92,7 +92,7 @@
   // potentially long-running callback is executing. |alarm_cancel| uses this
   // mutex to provide a guarantee to its caller that the callback will not be
   // in progress when it returns.
-  std::recursive_mutex* callback_mutex;
+  std::shared_ptr<std::recursive_mutex> callback_mutex;
   period_ms_t creation_time;
   period_ms_t period;
   period_ms_t deadline;
@@ -181,7 +181,8 @@
 
   alarm_t* ret = static_cast<alarm_t*>(osi_calloc(sizeof(alarm_t)));
 
-  ret->callback_mutex = new std::recursive_mutex;
+  std::shared_ptr<std::recursive_mutex> ptr(new std::recursive_mutex());
+  ret->callback_mutex = ptr;
   ret->is_periodic = is_periodic;
   ret->stats.name = osi_strdup(name);
 
@@ -198,7 +199,7 @@
   if (!alarm) return;
 
   alarm_cancel(alarm);
-  delete alarm->callback_mutex;
+
   osi_free((void*)alarm->stats.name);
   alarm->closure.~CancelableClosureInStruct();
   osi_free(alarm);
@@ -251,13 +252,15 @@
   CHECK(alarms != NULL);
   if (!alarm) return;
 
+  std::shared_ptr<std::recursive_mutex> local_mutex_ref = alarm->callback_mutex;
   {
     std::lock_guard<std::mutex> lock(alarms_mutex);
+    local_mutex_ref = alarm->callback_mutex;
     alarm_cancel_internal(alarm);
   }
 
   // If the callback for |alarm| is in progress, wait here until it completes.
-  std::lock_guard<std::recursive_mutex> lock(*alarm->callback_mutex);
+  std::lock_guard<std::recursive_mutex> lock(*local_mutex_ref);
 }
 
 // Internal implementation of canceling an alarm.
@@ -579,7 +582,10 @@
     alarm->queue = NULL;
   }
 
-  std::lock_guard<std::recursive_mutex> cb_lock(*alarm->callback_mutex);
+  // Increment the reference count of the mutex so it doesn't get freed
+  // before the callback gets finished executing.
+  std::shared_ptr<std::recursive_mutex> local_mutex_ref = alarm->callback_mutex;
+  std::lock_guard<std::recursive_mutex> cb_lock(*local_mutex_ref);
   lock.unlock();
 
   // Update the statistics

osi/test/alarm_test.cc

diff --git a/osi/test/alarm_test.cc b/osi/test/alarm_test.cc
index f7f473d..0066e19 100644
--- a/osi/test/alarm_test.cc
+++ b/osi/test/alarm_test.cc
@@ -364,3 +364,17 @@
   }
   alarm_cleanup();
 }
+
+static void remove_cb(void* data) {
+  alarm_free((alarm_t*)data);
+  semaphore_post(semaphore);
+}
+
+TEST_F(AlarmTest, test_delete_during_callback) {
+  for (int i = 0; i < 1000; ++i) {
+    alarm_t* alarm = alarm_new("alarm_test.test_delete_during_callback");
+    alarm_set(alarm, 0, remove_cb, alarm);
+    semaphore_wait(semaphore);
+  }
+  alarm_cleanup();
+}