Fix infinite loop when AllTargetsLost called with connected sessions.

PiperOrigin-RevId: 814725810
This commit is contained in:
Francis Tsui
2025-10-03 09:41:29 -07:00
committed by Copybara-Service
parent c65dc1abbb
commit d225c0dcbc
2 changed files with 84 additions and 6 deletions
+15 -6
View File
@@ -22,6 +22,7 @@
#include <optional>
#include <string>
#include <utility>
#include <vector>
#include "absl/base/nullability.h"
#include "absl/functional/any_invocable.h"
@@ -203,6 +204,7 @@ std::optional<ShareTarget> OutgoingTargetsManager::RemoveTarget(
if (session_it == outgoing_share_session_map_.end()) {
LOG(ERROR) << __func__ << ": share_target.id=" << share_target_id
<< " not found in outgoing share session map.";
outgoing_target_id_map_.erase(it);
return std::nullopt;
}
if (!close_connected && session_it->second.IsConnected()) {
@@ -307,16 +309,23 @@ OutgoingShareSession* OutgoingTargetsManager::GetOutgoingShareSession(
void OutgoingTargetsManager::AllTargetsLost(absl::Duration retention) {
VLOG(1) << "Move all outgoing share targets to discovery cache.";
while (!outgoing_target_id_map_.empty()) {
OnShareTargetLost(outgoing_target_id_map_.begin()->first, retention);
std::vector<std::string> endpoint_ids_to_remove;
endpoint_ids_to_remove.reserve(outgoing_target_id_map_.size());
for (const auto& [endpoint_id, share_target_id] : outgoing_target_id_map_) {
endpoint_ids_to_remove.push_back(endpoint_id);
}
for (const auto& endpoint_id : endpoint_ids_to_remove) {
OnShareTargetLost(endpoint_id, retention);
}
}
void OutgoingTargetsManager::Cleanup() {
while (!outgoing_target_id_map_.empty()) {
// Latch endpoint_id here since RemoveTarget() will remove the entry from
// the map.
std::string endpoint_id = outgoing_target_id_map_.begin()->first;
std::vector<std::string> endpoint_ids_to_remove;
endpoint_ids_to_remove.reserve(outgoing_target_id_map_.size());
for (const auto& [endpoint_id, share_target_id] : outgoing_target_id_map_) {
endpoint_ids_to_remove.push_back(endpoint_id);
}
for (const auto& endpoint_id : endpoint_ids_to_remove) {
RemoveTarget(endpoint_id, /*close_connected=*/true);
}
discovery_cache_.clear();
+69
View File
@@ -657,5 +657,74 @@ TEST_F(OutgoingTargetsManagerTest, CleanupClosesConnectedSessions) {
nullptr);
}
TEST_F(OutgoingTargetsManagerTest, AllTargetsLostClosesConnectedSessions) {
constexpr int kShareTargetId = 1234;
constexpr absl::string_view kEndpointId = "endpoint_id";
ShareTarget target;
target.id = kShareTargetId;
ShareTarget disabled_target = target;
disabled_target.receive_disabled = true;
{
InSequence s;
EXPECT_CALL(share_target_discovered_callback_, Call).Times(1);
EXPECT_CALL(share_target_updated_callback_, Call)
.WillOnce([&](const ShareTarget& share_target) {
EXPECT_EQ(share_target, disabled_target);
});
EXPECT_CALL(share_target_lost_callback_, Call).Times(0);
EXPECT_CALL(transfer_update_callback_, Call).Times(0);
}
outgoing_targets_manager_.OnShareTargetDiscovered(
target, kEndpointId, /*certificate=*/std::nullopt);
EXPECT_NE(outgoing_targets_manager_.GetOutgoingShareSession(kShareTargetId),
nullptr);
outgoing_targets_manager_.AllTargetsLost(Seconds(10));
bool has_targets = false;
outgoing_targets_manager_.ForEachShareTarget(
[&](const ShareTarget& share_target) {
has_targets = true;
EXPECT_EQ(share_target, disabled_target);
});
EXPECT_TRUE(has_targets);
EXPECT_EQ(outgoing_targets_manager_.GetOutgoingShareSession(kShareTargetId),
nullptr);
}
TEST_F(OutgoingTargetsManagerTest, AllTargetsLostConnectedSessionsNotClosed) {
constexpr int kShareTargetId = 1234;
constexpr absl::string_view kEndpointId = "endpoint_id";
ShareTarget target;
target.id = kShareTargetId;
{
InSequence s;
EXPECT_CALL(share_target_discovered_callback_, Call).Times(1);
EXPECT_CALL(share_target_updated_callback_, Call).Times(0);
EXPECT_CALL(share_target_lost_callback_, Call).Times(0);
EXPECT_CALL(transfer_update_callback_, Call).Times(0);
}
outgoing_targets_manager_.OnShareTargetDiscovered(
target, kEndpointId, /*certificate=*/std::nullopt);
OutgoingShareSession* session =
outgoing_targets_manager_.GetOutgoingShareSession(kShareTargetId);
ASSERT_NE(session, nullptr);
NearbyConnectionImpl connection(device_info_);
session->OnConnectResult(&connection, Status::kSuccess);
ASSERT_TRUE(session->IsConnected());
outgoing_targets_manager_.AllTargetsLost(Seconds(10));
bool has_targets = false;
outgoing_targets_manager_.ForEachShareTarget(
[&](const ShareTarget& share_target) {
has_targets = true;
EXPECT_EQ(share_target, target);
});
EXPECT_TRUE(has_targets);
EXPECT_NE(outgoing_targets_manager_.GetOutgoingShareSession(kShareTargetId),
nullptr);
}
} // namespace
} // namespace nearby::sharing