Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f7d04c6f authored by Pavlin Radoslavov's avatar Pavlin Radoslavov
Browse files

Update alarm_unregister_processing_queue() to cancel scheduled alarms 

Update the alarm_unregister_processing_queue() implementation
so it cancels all alarms that are scheduled on the corresponding
queue.
This fixes a race condition during Bluetooth shutdown: if an alarm
expires right after an alarm processing queue is invalidated,
the alarm processing would try to use the invalidated queue.

Added the corresponding unit tests.

Also, added a missing call to alarm_unregister_processing_queue().

Bug: 26982349
Change-Id: I09a111e8080b6dbc354dffa03a487f7a8c578ce6
parent 9221a9f0

system/bta/sys/bta_sys_main.c

+1 −0
Original line number Diff line number Diff line
@@ -193,6 +193,7 @@ void bta_sys_init(void) 
}



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;

}

system/osi/include/alarm.h

+2 −2
Original line number Diff line number Diff line
@@ -101,8 +101,8 @@ bool alarm_is_scheduled(const alarm_t *alarm); 
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.

system/osi/src/alarm.c

+28 −6
Original line number Diff line number Diff line
@@ -126,6 +126,7 @@ static period_ms_t now(void); 
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 @@ void alarm_cancel(alarm_t *alarm) { 
    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 @@ void alarm_cancel(alarm_t *alarm) { 
  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_register_processing_queue(fixed_queue_t *queue, thread_t *thread) { 
}



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,

system/osi/test/alarm_test.cpp

+117 −0
Original line number Diff line number Diff line
@@ -336,6 +336,123 @@ TEST_F(AlarmTest, test_callback_ordering_on_queue) { 
  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) {