Merge "Update alarm_unregister_processing_queue() to cancel scheduled alarms"
diff --git a/bta/sys/bta_sys_main.c b/bta/sys/bta_sys_main.c
index f0fbe52..af32cc9 100644
--- a/bta/sys/bta_sys_main.c
+++ b/bta/sys/bta_sys_main.c
@@ -193,6 +193,7 @@
}
void bta_sys_free(void) {
+ alarm_unregister_processing_queue(btu_bta_alarm_queue);
fixed_queue_free(btu_bta_alarm_queue, NULL);
btu_bta_alarm_queue = NULL;
}
diff --git a/osi/include/alarm.h b/osi/include/alarm.h
index e1089a4..3623865 100644
--- a/osi/include/alarm.h
+++ b/osi/include/alarm.h
@@ -105,8 +105,8 @@
void alarm_register_processing_queue(fixed_queue_t *queue, thread_t *thread);
// Unregisters |queue| for processing alarm callbacks on whichever thread
-// it is registered with. |queue| may not be NULL.
-// This function is idempotent.
+// it is registered with. All alarms currently set for execution on |queue|
+// will be canceled. |queue| may not be NULL. This function is idempotent.
void alarm_unregister_processing_queue(fixed_queue_t *queue);
// Figure out how much time until next expiration.
@@ -125,4 +125,4 @@
#ifdef __cplusplus
}
-#endif
\ No newline at end of file
+#endif
diff --git a/osi/src/alarm.c b/osi/src/alarm.c
index 669366d..69ded69 100644
--- a/osi/src/alarm.c
+++ b/osi/src/alarm.c
@@ -126,6 +126,7 @@
static void alarm_set_internal(alarm_t *alarm, period_ms_t period,
alarm_callback_t cb, void *data,
fixed_queue_t *queue);
+static void alarm_cancel_internal(alarm_t *alarm);
static void remove_pending_alarm(alarm_t *alarm);
static void schedule_next_instance(alarm_t *alarm);
static void reschedule_root_alarm(void);
@@ -260,7 +261,17 @@
return;
pthread_mutex_lock(&monitor);
+ alarm_cancel_internal(alarm);
+ pthread_mutex_unlock(&monitor);
+ // If the callback for |alarm| is in progress, wait here until it completes.
+ pthread_mutex_lock(&alarm->callback_lock);
+ pthread_mutex_unlock(&alarm->callback_lock);
+}
+
+// Internal implementation of canceling an alarm.
+// The caller must hold the |monitor| lock.
+static void alarm_cancel_internal(alarm_t *alarm) {
bool needs_reschedule = (!list_is_empty(alarms) && list_front(alarms) == alarm);
remove_pending_alarm(alarm);
@@ -270,15 +281,10 @@
alarm->callback = NULL;
alarm->data = NULL;
alarm->stats.canceled_count++;
+ alarm->queue = NULL;
if (needs_reschedule)
reschedule_root_alarm();
-
- pthread_mutex_unlock(&monitor);
-
- // If the callback for |alarm| is in progress, wait here until it completes.
- pthread_mutex_lock(&alarm->callback_lock);
- pthread_mutex_unlock(&alarm->callback_lock);
}
bool alarm_is_scheduled(const alarm_t *alarm) {
@@ -557,7 +563,23 @@
}
void alarm_unregister_processing_queue(fixed_queue_t *queue) {
+ assert(alarms != NULL);
+ assert(queue != NULL);
+
fixed_queue_unregister_dequeue(queue);
+
+ // Cancel all alarms that are using this queue
+ pthread_mutex_lock(&monitor);
+ for (list_node_t *node = list_begin(alarms); node != list_end(alarms); ) {
+ alarm_t *alarm = (alarm_t *)list_node(node);
+ node = list_next(node);
+ // TODO: Each module is responsible for tearing down its alarms; currently,
+ // this is not the case. In the future, this check should be replaced by
+ // an assert.
+ if (alarm->queue == queue)
+ alarm_cancel_internal(alarm);
+ }
+ pthread_mutex_unlock(&monitor);
}
static void alarm_queue_ready(fixed_queue_t *queue,
diff --git a/osi/test/alarm_test.cpp b/osi/test/alarm_test.cpp
index 954468c..e4436aa 100644
--- a/osi/test/alarm_test.cpp
+++ b/osi/test/alarm_test.cpp
@@ -336,6 +336,123 @@
thread_free(thread);
}
+// Test whether unregistering a processing queue cancels all timers using
+// that queue.
+TEST_F(AlarmTest, test_unregister_processing_queue) {
+ alarm_t *alarms[100];
+ fixed_queue_t *queue = fixed_queue_new(SIZE_MAX);
+ thread_t *thread =
+ thread_new("timers.test_unregister_processing_queue.thread");
+
+ alarm_register_processing_queue(queue, thread);
+
+ for (int i = 0; i < 100; i++) {
+ const std::string alarm_name =
+ "alarm_test.test_unregister_processing_queue[" +
+ std::to_string(i) + "]";
+ alarms[i] = alarm_new(alarm_name.c_str());
+ }
+
+ // Schedule half of the timers to expire soon, and the rest far in the future
+ for (int i = 0; i < 50; i++) {
+ alarm_set_on_queue(alarms[i], 100, ordered_cb, INT_TO_PTR(i), queue);
+ }
+ for (int i = 50; i < 100; i++) {
+ alarm_set_on_queue(alarms[i], 1000 * 1000, ordered_cb, INT_TO_PTR(i), queue);
+ }
+
+ // Wait until half of the timers have expired
+ for (int i = 1; i <= 50; i++) {
+ semaphore_wait(semaphore);
+ EXPECT_GE(cb_counter, i);
+ }
+ EXPECT_EQ(cb_counter, 50);
+ EXPECT_EQ(cb_misordered_counter, 0);
+
+ // Test that only the expired timers are not scheduled
+ for (int i = 0; i < 50; i++) {
+ EXPECT_FALSE(alarm_is_scheduled(alarms[i]));
+ }
+ for (int i = 50; i < 100; i++) {
+ EXPECT_TRUE(alarm_is_scheduled(alarms[i]));
+ }
+
+ alarm_unregister_processing_queue(queue);
+
+ // Test that none of the timers are scheduled
+ for (int i = 0; i < 100; i++) {
+ EXPECT_FALSE(alarm_is_scheduled(alarms[i]));
+ }
+
+ for (int i = 0; i < 100; i++) {
+ alarm_free(alarms[i]);
+ }
+
+ EXPECT_FALSE(WakeLockHeld());
+
+ fixed_queue_free(queue, NULL);
+ thread_free(thread);
+}
+
+// Test whether unregistering a processing queue cancels all periodic timers
+// using that queue.
+TEST_F(AlarmTest, test_periodic_unregister_processing_queue) {
+ alarm_t *alarms[5];
+ fixed_queue_t *queue = fixed_queue_new(SIZE_MAX);
+ thread_t *thread =
+ thread_new("timers.test_periodic_unregister_processing_queue.thread");
+
+ alarm_register_processing_queue(queue, thread);
+
+ for (int i = 0; i < 5; i++) {
+ const std::string alarm_name =
+ "alarm_test.test_periodic_unregister_processing_queue[" +
+ std::to_string(i) + "]";
+ alarms[i] = alarm_new_periodic(alarm_name.c_str());
+ }
+
+ // Schedule each of the timers with different period
+ for (int i = 0; i < 5; i++) {
+ alarm_set_on_queue(alarms[i], 20 + i, cb, INT_TO_PTR(i), queue);
+ }
+ EXPECT_EQ(cb_counter, 0);
+ EXPECT_TRUE(WakeLockHeld());
+
+ for (int i = 1; i <= 20; i++) {
+ semaphore_wait(semaphore);
+
+ EXPECT_GE(cb_counter, i);
+ EXPECT_TRUE(WakeLockHeld());
+ }
+
+ // Test that all timers are still scheduled
+ for (int i = 0; i < 5; i++) {
+ EXPECT_TRUE(alarm_is_scheduled(alarms[i]));
+ }
+
+ alarm_unregister_processing_queue(queue);
+
+ int saved_cb_counter = cb_counter;
+
+ // Test that none of the timers are scheduled
+ for (int i = 0; i < 5; i++) {
+ EXPECT_FALSE(alarm_is_scheduled(alarms[i]));
+ }
+
+ // Sleep for 500ms and test again that the cb_counter hasn't been modified
+ usleep(500 * 1000);
+ EXPECT_TRUE(cb_counter == saved_cb_counter);
+
+ for (int i = 0; i < 5; i++) {
+ alarm_free(alarms[i]);
+ }
+
+ EXPECT_FALSE(WakeLockHeld());
+
+ fixed_queue_free(queue, NULL);
+ thread_free(thread);
+}
+
// Try to catch any race conditions between the timer callback and |alarm_free|.
TEST_F(AlarmTest, test_callback_free_race) {
for (int i = 0; i < 1000; ++i) {
