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

Commit 96b96ee2 authored by Jeremy Wu's avatar Jeremy Wu
Browse files

Floss: access message_loop_thread_ only via worker thread 

|RepeatingTimer|s hold a weak pointer to the static worker thread
|btif_a2dp_source_thread|, and is only meant to dereference it on the
same thread (as the worker thread).

However, there is much of abusing its ownership, trying to dereference
it from either the main thread or uipc thread, for the sake of using it
as a condition to determine if audio is streaming. This alone is prone
to race conditions, and causes immediate crashes thrown by libchrome.

In this CL, we ensure dereference of the weak pointer in question is
only invoked via the worker thread:

- Let |btif_a2dp_source_audio_tx_stop_event|, the worker thread
  function, decide if it |is_streaming|
- Remove redundant calls to |media_alarm.IsScheduled|
- Remove |is_streaming| checks that are not necessary

Bug: 257166696
Tag: #floss
Test: Build and verify
Change-Id: Ibdfa24ce10b254b8ac5717b7ca64e95bf933f832
parent f9ce5d9c

system/audio_hal_interface/a2dp_encoding_host.cc

+2 −9
Original line number Diff line number Diff line
@@ -65,15 +65,8 @@ static void btif_a2dp_data_cb([[maybe_unused]] tUIPC_CH_ID ch_id, 
      break;



    case UIPC_CLOSE_EVT:

      /*

       * Send stop request only if we are actively streaming and haven't

       * received a stop request. Potentially, the audioflinger detached

       * abnormally.

       */

      if (btif_a2dp_source_is_streaming()) {

      /* Post stop event and wait for audio path to stop */

      btif_av_stream_stop(RawAddress::kEmpty);

      }

      break;



    default:

system/btif/src/btif_a2dp_control.cc

+2 −20
Original line number Diff line number Diff line
@@ -93,12 +93,6 @@ static tA2DP_CTRL_ACK btif_a2dp_control_on_start() { 
    return A2DP_CTRL_ACK_INCALL_FAILURE;

  }



  if (btif_a2dp_source_is_streaming()) {

    APPL_TRACE_WARNING("%s: A2DP command start while source is streaming",

                       __func__);

    return A2DP_CTRL_ACK_FAILURE;

  }



  if (btif_av_stream_ready()) {

    /* Setup audio data channel listener */

    UIPC_Open(*a2dp_uipc, UIPC_CH_ID_AV_AUDIO, btif_a2dp_data_cb,
@@ -128,11 +122,6 @@ static tA2DP_CTRL_ACK btif_a2dp_control_on_start() { 
}



static tA2DP_CTRL_ACK btif_a2dp_control_on_stop() {

  if (btif_av_get_peer_sep() == AVDT_TSEP_SNK &&

      !btif_a2dp_source_is_streaming()) {

    /* We are already stopped, just ack back */

    return A2DP_CTRL_ACK_SUCCESS;

  }

  btif_av_stream_stop(RawAddress::kEmpty);

  return A2DP_CTRL_ACK_SUCCESS;

}
@@ -394,15 +383,8 @@ static void btif_a2dp_data_cb(UNUSED_ATTR tUIPC_CH_ID ch_id, 
    case UIPC_CLOSE_EVT:

      APPL_TRACE_EVENT("%s: ## AUDIO PATH DETACHED ##", __func__);

      btif_a2dp_command_ack(A2DP_CTRL_ACK_SUCCESS);

      /*

       * Send stop request only if we are actively streaming and haven't

       * received a stop request. Potentially, the audioflinger detached

       * abnormally.

       */

      if (btif_a2dp_source_is_streaming()) {

      /* Post stop event and wait for audio path to stop */

      btif_av_stream_stop(RawAddress::kEmpty);

      }

      break;



    default:

system/btif/src/btif_a2dp_source.cc

+13 −21
Original line number Diff line number Diff line
@@ -422,18 +422,14 @@ static void btif_a2dp_source_start_session_delayed( 
bool btif_a2dp_source_restart_session(const RawAddress& old_peer_address,

                                      const RawAddress& new_peer_address,

                                      std::promise<void> peer_ready_promise) {

  bool is_streaming = btif_a2dp_source_cb.media_alarm.IsScheduled();

  LOG(INFO) << __func__ << ": old_peer_address=" << old_peer_address

            << " new_peer_address=" << new_peer_address

            << " is_streaming=" << logbool(is_streaming)

            << " state=" << btif_a2dp_source_cb.StateStr();



  CHECK(!new_peer_address.IsEmpty());



  // Must stop first the audio streaming

  if (is_streaming) {

  btif_a2dp_source_stop_audio_req();

  }



  // If the old active peer was valid, end the old session.

  // Otherwise, time to startup the A2DP Source processing.
@@ -542,6 +538,7 @@ bool btif_a2dp_source_media_task_is_shutting_down(void) { 
  return (btif_a2dp_source_cb.State() == BtifA2dpSource::kStateShuttingDown);

}



// This runs on worker thread

bool btif_a2dp_source_is_streaming(void) {

  return btif_a2dp_source_cb.media_alarm.IsScheduled();

}
@@ -602,10 +599,8 @@ static void btif_a2dp_source_setup_codec_delayed( 


static void btif_a2dp_source_cleanup_codec() {

  LOG_INFO("%s: state=%s", __func__, btif_a2dp_source_cb.StateStr().c_str());

  if (btif_a2dp_source_is_streaming()) {

  // Must stop media task first before cleaning up the encoder

  btif_a2dp_source_stop_audio_req();

  }

  btif_a2dp_source_thread.DoInThread(

      FROM_HERE, base::Bind(&btif_a2dp_source_cleanup_codec_delayed));

}
@@ -787,9 +782,7 @@ void btif_a2dp_source_set_tx_flush(bool enable) { 
}



static void btif_a2dp_source_audio_tx_start_event(void) {

  LOG_INFO(

      "%s: media_alarm is %s, streaming %s state=%s", __func__,

      btif_a2dp_source_cb.media_alarm.IsScheduled() ? "running" : "stopped",

  LOG_INFO("%s: streaming %s state=%s", __func__,

           btif_a2dp_source_is_streaming() ? "true" : "false",

           btif_a2dp_source_cb.StateStr().c_str());
@@ -834,13 +827,12 @@ static void btif_a2dp_source_audio_tx_start_event(void) { 
}



static void btif_a2dp_source_audio_tx_stop_event(void) {

  LOG_INFO(

      "%s: media_alarm is %s, streaming %s state=%s", __func__,

      btif_a2dp_source_cb.media_alarm.IsScheduled() ? "running" : "stopped",

  LOG_INFO("%s: streaming %s state=%s", __func__,

           btif_a2dp_source_is_streaming() ? "true" : "false",

           btif_a2dp_source_cb.StateStr().c_str());



  if (btif_av_is_a2dp_offload_running()) return;

  if (!btif_a2dp_source_is_streaming()) return;



  btif_a2dp_source_cb.stats.session_end_us =

      bluetooth::common::time_get_os_boottime_us();
@@ -905,7 +897,7 @@ static void btif_a2dp_source_audio_handle_timer(void) { 


  log_tstamps_us("A2DP Source tx scheduling timer", timestamp_us);



  if (!btif_a2dp_source_cb.media_alarm.IsScheduled()) {

  if (!btif_a2dp_source_is_streaming()) {

    LOG_ERROR("%s: ERROR Media task Scheduled after Suspend", __func__);

    return;

  }
@@ -958,7 +950,7 @@ static bool btif_a2dp_source_enqueue_callback(BT_HDR* p_buf, size_t frames_n, 
  btif_a2dp_control_log_bytes_read(bytes_read);



  /* Check if timer was stopped (media task stopped) */

  if (!btif_a2dp_source_cb.media_alarm.IsScheduled()) {

  if (!btif_a2dp_source_is_streaming()) {

    osi_free(p_buf);

    return false;

  }