L2CAP SignallingManager: Store command_just_sent
Instead of storing it in pending_commands_ *after* a command is sent, we
use a separate variable command_just_sent_ to store it, to avoid state
corruption
Test: bluetooth_test_gd and run_cert.sh
Bug: 145622504
Change-Id: I5992b3545c04ec2bc36fcc43ae84b81a6b26eb8f
diff --git a/gd/l2cap/classic/internal/signalling_manager.cc b/gd/l2cap/classic/internal/signalling_manager.cc
index 869094e..de2f5fd 100644
--- a/gd/l2cap/classic/internal/signalling_manager.cc
+++ b/gd/l2cap/classic/internal/signalling_manager.cc
@@ -56,18 +56,11 @@
}
void ClassicSignallingManager::OnCommandReject(CommandRejectView command_reject_view) {
- if (pending_commands_.empty()) {
+ if (command_just_sent_.signal_id_ != command_reject_view.GetIdentifier() ||
+ command_just_sent_.command_code_ != command_reject_view.GetCode()) {
LOG_WARN("Unexpected command reject: no pending request");
return;
}
- auto last_sent_command = std::move(pending_commands_.front());
- pending_commands_.pop();
-
- SignalId signal_id = command_reject_view.GetIdentifier();
- if (last_sent_command.signal_id_ != signal_id) {
- LOG_WARN("Unknown command reject");
- return;
- }
alarm_.Cancel();
handle_send_next_command();
@@ -78,7 +71,7 @@
PendingCommand pending_command = {next_signal_id_, CommandCode::CONNECTION_REQUEST, psm, local_cid, {}, {}, {}};
next_signal_id_++;
pending_commands_.push(std::move(pending_command));
- if (pending_commands_.size() == 1) {
+ if (command_just_sent_.signal_id_ == kInvalidSignalId) {
handle_send_next_command();
}
}
@@ -89,7 +82,7 @@
std::move(config)};
next_signal_id_++;
pending_commands_.push(std::move(pending_command));
- if (pending_commands_.size() == 1) {
+ if (command_just_sent_.signal_id_ == kInvalidSignalId) {
handle_send_next_command();
}
}
@@ -99,7 +92,7 @@
next_signal_id_, CommandCode::DISCONNECTION_REQUEST, {}, local_cid, remote_cid, {}, {}};
next_signal_id_++;
pending_commands_.push(std::move(pending_command));
- if (pending_commands_.size() == 1) {
+ if (command_just_sent_.signal_id_ == kInvalidSignalId) {
handle_send_next_command();
}
}
@@ -108,7 +101,7 @@
PendingCommand pending_command = {next_signal_id_, CommandCode::INFORMATION_REQUEST, {}, {}, {}, type, {}};
next_signal_id_++;
pending_commands_.push(std::move(pending_command));
- if (pending_commands_.size() == 1) {
+ if (command_just_sent_.signal_id_ == kInvalidSignalId) {
handle_send_next_command();
}
}
@@ -200,28 +193,26 @@
void ClassicSignallingManager::OnConnectionResponse(SignalId signal_id, Cid remote_cid, Cid cid,
ConnectionResponseResult result, ConnectionResponseStatus status) {
- if (pending_commands_.empty()) {
- LOG_WARN("Unexpected response: no pending request");
+ if (command_just_sent_.signal_id_ != signal_id ||
+ command_just_sent_.command_code_ != CommandCode::CONNECTION_REQUEST) {
+ LOG_WARN("Unexpected response: no pending request. Expected signal id %d type %s, got %d",
+ command_just_sent_.signal_id_.Value(), CommandCodeText(command_just_sent_.command_code_).data(),
+ signal_id.Value());
return;
}
- auto last_sent_command = std::move(pending_commands_.front());
- pending_commands_.pop();
- if (last_sent_command.signal_id_ != signal_id || last_sent_command.command_code_ != CommandCode::CONNECTION_REQUEST) {
- LOG_WARN("Received unexpected connection response");
- return;
- }
- if (last_sent_command.source_cid_ != cid) {
- LOG_WARN("SCID doesn't match: expected %d, received %d", last_sent_command.source_cid_, cid);
+ if (command_just_sent_.source_cid_ != cid) {
+ LOG_WARN("SCID doesn't match: expected %d, received %d", command_just_sent_.source_cid_, cid);
handle_send_next_command();
return;
}
+ command_just_sent_.signal_id_ = kInvalidSignalId;
alarm_.Cancel();
if (result != ConnectionResponseResult::SUCCESS) {
link_->OnOutgoingConnectionRequestFail(cid);
handle_send_next_command();
return;
}
- Psm pending_psm = last_sent_command.psm_;
+ Psm pending_psm = command_just_sent_.psm_;
auto new_channel = link_->AllocateReservedDynamicChannel(cid, pending_psm, remote_cid, {});
if (new_channel == nullptr) {
LOG_WARN("Can't allocate dynamic channel");
@@ -336,14 +327,14 @@
void ClassicSignallingManager::OnConfigurationResponse(SignalId signal_id, Cid cid, Continuation is_continuation,
ConfigurationResponseResult result,
std::vector<std::unique_ptr<ConfigurationOption>> options) {
- if (pending_commands_.empty()) {
- LOG_WARN("Unexpected response: no pending request");
+ if (command_just_sent_.signal_id_ != signal_id ||
+ command_just_sent_.command_code_ != CommandCode::CONFIGURATION_REQUEST) {
+ LOG_WARN("Unexpected response: no pending request. Expected signal id %d type %s, got %d",
+ command_just_sent_.signal_id_.Value(), CommandCodeText(command_just_sent_.command_code_).data(),
+ signal_id.Value());
return;
}
- auto last_sent_command = std::move(pending_commands_.front());
- pending_commands_.pop();
-
auto channel = channel_allocator_->FindChannelByCid(cid);
if (channel == nullptr) {
LOG_WARN("Configuration request for an unknown channel");
@@ -413,18 +404,15 @@
}
void ClassicSignallingManager::OnDisconnectionResponse(SignalId signal_id, Cid remote_cid, Cid cid) {
- if (pending_commands_.empty()) {
- LOG_WARN("Unexpected response: no pending request");
+ if (command_just_sent_.signal_id_ != signal_id ||
+ command_just_sent_.command_code_ != CommandCode::DISCONNECTION_REQUEST) {
+ LOG_WARN("Unexpected response: no pending request. Expected signal id %d type %s, got %d",
+ command_just_sent_.signal_id_.Value(), CommandCodeText(command_just_sent_.command_code_).data(),
+ signal_id.Value());
return;
}
- auto last_sent_command = std::move(pending_commands_.front());
- pending_commands_.pop();
- alarm_.Cancel();
- if (last_sent_command.signal_id_ != signal_id ||
- last_sent_command.command_code_ != CommandCode::DISCONNECTION_REQUEST) {
- return;
- }
+ alarm_.Cancel();
auto channel = channel_allocator_->FindChannelByCid(cid);
if (channel == nullptr) {
@@ -447,14 +435,10 @@
}
void ClassicSignallingManager::OnEchoResponse(SignalId signal_id, const PacketView<kLittleEndian>& packet) {
- if (pending_commands_.empty()) {
- LOG_WARN("Unexpected response: no pending request");
- return;
- }
- auto last_sent_command = std::move(pending_commands_.front());
- pending_commands_.pop();
-
- if (last_sent_command.signal_id_ != signal_id || last_sent_command.command_code_ != CommandCode::ECHO_REQUEST) {
+ if (command_just_sent_.signal_id_ != signal_id || command_just_sent_.command_code_ != CommandCode::ECHO_REQUEST) {
+ LOG_WARN("Unexpected response: no pending request. Expected signal id %d type %s, got %d",
+ command_just_sent_.signal_id_.Value(), CommandCodeText(command_just_sent_.command_code_).data(),
+ signal_id.Value());
return;
}
LOG_INFO("Echo response received");
@@ -487,15 +471,11 @@
}
void ClassicSignallingManager::OnInformationResponse(SignalId signal_id, const InformationResponseView& response) {
- if (pending_commands_.empty()) {
- LOG_WARN("Unexpected response: no pending request");
- return;
- }
- auto last_sent_command = std::move(pending_commands_.front());
- pending_commands_.pop();
-
- if (last_sent_command.signal_id_ != signal_id ||
- last_sent_command.command_code_ != CommandCode::INFORMATION_REQUEST) {
+ if (command_just_sent_.signal_id_ != signal_id ||
+ command_just_sent_.command_code_ != CommandCode::INFORMATION_REQUEST) {
+ LOG_WARN("Unexpected response: no pending request. Expected signal id %d type %s, got %d",
+ command_just_sent_.signal_id_.Value(), CommandCodeText(command_just_sent_.command_code_).data(),
+ signal_id.Value());
return;
}
@@ -659,20 +639,18 @@
void ClassicSignallingManager::on_command_timeout() {
LOG_WARN("Response time out");
- if (pending_commands_.empty()) {
+ if (command_just_sent_.signal_id_ == kInvalidSignalId) {
LOG_ERROR("No pending command");
return;
}
- auto last_sent_command = std::move(pending_commands_.front());
- pending_commands_.pop();
- switch (last_sent_command.command_code_) {
+ switch (command_just_sent_.command_code_) {
case CommandCode::CONNECTION_REQUEST: {
- link_->OnOutgoingConnectionRequestFail(last_sent_command.source_cid_);
+ link_->OnOutgoingConnectionRequestFail(command_just_sent_.source_cid_);
break;
}
case CommandCode::CONFIGURATION_REQUEST: {
- SendDisconnectionRequest(last_sent_command.source_cid_, last_sent_command.destination_cid_);
+ SendDisconnectionRequest(command_just_sent_.source_cid_, command_just_sent_.destination_cid_);
break;
}
default:
@@ -682,18 +660,20 @@
}
void ClassicSignallingManager::handle_send_next_command() {
+ command_just_sent_.signal_id_ = kInvalidSignalId;
if (pending_commands_.empty()) {
return;
}
- auto& last_sent_command = pending_commands_.front();
+ command_just_sent_ = std::move(pending_commands_.front());
+ pending_commands_.pop();
- auto signal_id = last_sent_command.signal_id_;
- auto psm = last_sent_command.psm_;
- auto source_cid = last_sent_command.source_cid_;
- auto destination_cid = last_sent_command.destination_cid_;
- auto info_type = last_sent_command.info_type_;
- auto config = std::move(last_sent_command.config_);
- switch (last_sent_command.command_code_) {
+ auto signal_id = command_just_sent_.signal_id_;
+ auto psm = command_just_sent_.psm_;
+ auto source_cid = command_just_sent_.source_cid_;
+ auto destination_cid = command_just_sent_.destination_cid_;
+ auto info_type = command_just_sent_.info_type_;
+ auto config = std::move(command_just_sent_.config_);
+ switch (command_just_sent_.command_code_) {
case CommandCode::CONNECTION_REQUEST: {
auto builder = ConnectionRequestBuilder::Create(signal_id.Value(), psm, source_cid);
enqueue_buffer_->Enqueue(std::move(builder), handler_);
@@ -724,7 +704,7 @@
break;
}
default:
- LOG_WARN("Unsupported command code 0x%x", static_cast<int>(last_sent_command.command_code_));
+ LOG_WARN("Unsupported command code 0x%x", static_cast<int>(command_just_sent_.command_code_));
}
}
diff --git a/gd/l2cap/classic/internal/signalling_manager.h b/gd/l2cap/classic/internal/signalling_manager.h
index aa2135f..0f706e4 100644
--- a/gd/l2cap/classic/internal/signalling_manager.h
+++ b/gd/l2cap/classic/internal/signalling_manager.h
@@ -41,7 +41,7 @@
namespace internal {
struct PendingCommand {
- SignalId signal_id_;
+ SignalId signal_id_ = kInvalidSignalId;
CommandCode command_code_;
Psm psm_;
Cid source_cid_;
@@ -113,6 +113,7 @@
FixedChannelServiceManagerImpl* fixed_service_manager_;
std::unique_ptr<os::EnqueueBuffer<packet::BasePacketBuilder>> enqueue_buffer_;
std::queue<PendingCommand> pending_commands_;
+ PendingCommand command_just_sent_;
os::Alarm alarm_;
SignalId next_signal_id_ = kInitialSignalId;
std::unordered_map<Cid, ChannelConfigurationState> channel_configuration_;
diff --git a/gd/l2cap/le/internal/signalling_manager.cc b/gd/l2cap/le/internal/signalling_manager.cc
index a67bd12..18b2e95 100644
--- a/gd/l2cap/le/internal/signalling_manager.cc
+++ b/gd/l2cap/le/internal/signalling_manager.cc
@@ -98,7 +98,6 @@
return;
}
alarm_.Cancel();
- command_just_sent_.signal_id_ = kInitialSignalId;
handle_send_next_command();
LOG_WARN("Command rejected");
@@ -365,7 +364,6 @@
LOG_ERROR("No pending command");
return;
}
- command_just_sent_.signal_id_ = kInvalidSignalId;
switch (command_just_sent_.command_code_) {
case LeCommandCode::CONNECTION_PARAMETER_UPDATE_REQUEST: {
link_->OnOutgoingConnectionRequestFail(command_just_sent_.source_cid_);
@@ -378,6 +376,7 @@
}
void LeSignallingManager::handle_send_next_command() {
+ command_just_sent_.signal_id_ = kInvalidSignalId;
if (pending_commands_.empty()) {
return;
}
diff --git a/gd/l2cap/le/internal/signalling_manager.h b/gd/l2cap/le/internal/signalling_manager.h
index 2ac16b8..c93812b 100644
--- a/gd/l2cap/le/internal/signalling_manager.h
+++ b/gd/l2cap/le/internal/signalling_manager.h
@@ -41,7 +41,7 @@
namespace internal {
struct PendingCommand {
- SignalId signal_id_ = kInitialSignalId;
+ SignalId signal_id_ = kInvalidSignalId;
LeCommandCode command_code_;
Psm psm_;
Cid source_cid_;
@@ -99,7 +99,7 @@
os::Handler* handler_;
Link* link_;
- [[maybe_unused]] l2cap::internal::DataPipelineManager* data_pipeline_manager_;
+ l2cap::internal::DataPipelineManager* data_pipeline_manager_;
std::shared_ptr<le::internal::FixedChannelImpl> signalling_channel_;
DynamicChannelServiceManagerImpl* dynamic_service_manager_;
l2cap::internal::DynamicChannelAllocator* channel_allocator_;