Skip to content

Commit

Permalink
Move disconnection timeout into OutgoingShareSession.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 707655553
  • Loading branch information
ftsui authored and copybara-github committed Dec 19, 2024
1 parent f7261d2 commit 05a284c
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 42 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/validate.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ jobs:
steps:
- uses: actions/checkout@v3
- name: Build Connections
run: CC=clang-15 CXX=clang-15++ bazel build --copt='-DGITHUB_BUILD' //connections:core
run: CC=clang-16 CXX=clang-16++ bazel build --copt='-DGITHUB_BUILD' //connections:core
- name: Build Presence
run: CC=clang-15 CXX=clang-15++ bazel build --copt='-DGITHUB_BUILD' //presence
run: CC=clang-16 CXX=clang-16++ bazel build --copt='-DGITHUB_BUILD' //presence
- name: Build Sharing
run: CC=clang-15 CXX=clang-15++ bazel build --verbose_failures --copt='-DGITHUB_BUILD' //sharing/proto/... //sharing/analytics
run: CC=clang-16 CXX=clang-16++ bazel build --verbose_failures --copt='-DGITHUB_BUILD' //sharing/proto/... //sharing/analytics

build-rust-linux:
name: Build Rust on Linux
Expand Down
23 changes: 1 addition & 22 deletions sharing/nearby_sharing_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,6 @@ void NearbySharingServiceImpl::Cleanup() {
last_outgoing_metadata_.reset();
locally_cancelled_share_target_ids_.clear();

disconnection_timeout_alarms_.clear();

is_scanning_ = false;
is_transferring_ = false;
is_receiving_files_ = false;
Expand Down Expand Up @@ -3092,24 +3090,8 @@ void NearbySharingServiceImpl::OnOutgoingPayloadTransferUpdates(
<< TransferMetadata::StatusToString(metadata->status());
}

// When kComplete is received from PayloadTracker, we need to wait for
// receiver to close connection before we know that the transfer has
// succeeded. Here we delay the kComplete update until receiver disconnects.
// A 1 min timer is setup so that if we do not receive disconnect from
// receiver, we assume the transfer has failed.
if (metadata->status() == TransferMetadata::Status::kComplete) {
session->DelayCompleteMetadata(*metadata);
auto timer = std::make_unique<ThreadTimer>(
*service_thread_, "disconnection_timeout_alarm",
kOutgoingDisconnectionDelay,
[this, share_target_id = session->share_target().id]() {
OutgoingShareSession* session =
GetOutgoingShareSession(share_target_id);
if (session != nullptr) {
session->DisconnectionTimeout();
}
});
disconnection_timeout_alarms_[session->endpoint_id()] = std::move(timer);
session->DelayComplete(*metadata);
return;
}
// Make sure to call this before calling Disconnect, or we risk losing some
Expand Down Expand Up @@ -3293,9 +3275,6 @@ bool NearbySharingServiceImpl::FindDuplicateInOutgoingShareTargets(
std::optional<ShareTarget>
NearbySharingServiceImpl::RemoveOutgoingShareTargetWithEndpointId(
absl::string_view endpoint_id) {
VLOG(1) << __func__ << ":Outgoing connection to " << endpoint_id
<< " disconnected, cancel disconnection timer";
disconnection_timeout_alarms_.erase(endpoint_id);
auto target_node = outgoing_share_target_map_.extract(endpoint_id);
if (target_node.empty()) {
LOG(WARNING) << __func__ << ": endpoint_id=" << endpoint_id
Expand Down
16 changes: 9 additions & 7 deletions sharing/outgoing_share_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ std::optional<Payload> OutgoingShareSession::ExtractNextPayload() {
return std::nullopt;
}

void OutgoingShareSession::DelayCompleteMetadata(
void OutgoingShareSession::DelayComplete(
const TransferMetadata& complete_metadata) {
LOG(INFO)
<< "Delay complete notification until receiver disconnects for target "
Expand All @@ -517,12 +517,14 @@ void OutgoingShareSession::DelayCompleteMetadata(
TransferMetadataBuilder::Clone(complete_metadata);
builder.set_status(TransferMetadata::Status::kInProgress);
UpdateTransferMetadata(builder.build());
}

void OutgoingShareSession::DisconnectionTimeout() {
VLOG(1) << "Disconnection delay timeed out for target " << share_target().id;
pending_complete_metadata_.reset();
Disconnect();
disconnection_timeout_ = std::make_unique<ThreadTimer>(
service_thread(), "disconnection_timeout_alarm",
kOutgoingDisconnectionDelay, [this]() {
VLOG(1) << "Disconnection delay timed out for target "
<< share_target().id;
pending_complete_metadata_.reset();
Disconnect();
});
}

void OutgoingShareSession::UpdateSessionForDedup(
Expand Down
14 changes: 9 additions & 5 deletions sharing/outgoing_share_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,13 @@ class OutgoingShareSession : public ShareSession {
// Called when all payloads have been sent.
void SendAttachmentsCompleted(const TransferMetadata& metadata);

// Cache the kComplete metadata in pending_complete_metadata_ and forward a
// modified copy that changes kComplete into kInProgress.
void DelayCompleteMetadata(const TransferMetadata& complete_metadata);
// Disconnect timeout expired without receiving client disconnect.
void DisconnectionTimeout();
// Wait for receiver to close connection before we know that the transfer has
// succeeded. Here we delay the `complete_metadata` update until receiver
// disconnects and forward a modified copy that changes kComplete into
// kInProgress.
// A 1 min timer is setup so that if we do not receive disconnect from
// receiver, we assume the transfer has failed.
void DelayComplete(const TransferMetadata& complete_metadata);
// Used only for OutgoingShareSession De-duplication.
void UpdateSessionForDedup(
const ShareTarget& share_target,
Expand Down Expand Up @@ -195,6 +197,8 @@ class OutgoingShareSession : public ShareSession {
std::unique_ptr<ThreadTimer> mutual_acceptance_timeout_;
std::optional<TransferMetadata> pending_complete_metadata_;
absl::Time connection_start_time_;
// Timeout waiting for remote disconnect in order to complete transfer.
std::unique_ptr<ThreadTimer> disconnection_timeout_;
};

} // namespace nearby::sharing
Expand Down
13 changes: 8 additions & 5 deletions sharing/outgoing_share_session_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,7 @@ TEST_F(OutgoingShareSessionTest, ProcessKeyVerificationResultSuccess) {
EXPECT_THAT(session_.os_type(), Eq(OSType::WINDOWS));
}

TEST_F(OutgoingShareSessionTest, DelayCompleteMetadataReceiverDisconnect) {
TEST_F(OutgoingShareSessionTest, DelayCompleteReceiverDisconnect) {
NearbyConnectionImpl connection(device_info_, &connections_manager_,
kEndpointId);
session_.set_session_id(1234);
Expand All @@ -893,14 +893,14 @@ TEST_F(OutgoingShareSessionTest, DelayCompleteMetadataReceiverDisconnect) {
EXPECT_CALL(transfer_metadata_callback_,
Call(_, HasStatus(TransferMetadata::Status::kInProgress)));

session_.DelayCompleteMetadata(complete_metadata);
session_.DelayComplete(complete_metadata);

EXPECT_CALL(transfer_metadata_callback_,
Call(_, HasStatus(TransferMetadata::Status::kComplete)));
session_.OnDisconnect();
}

TEST_F(OutgoingShareSessionTest, DelayCompleteMetadataDisconnectTimeout) {
TEST_F(OutgoingShareSessionTest, DelayCompleteDisconnectTimeout) {
NearbyConnectionImpl connection(device_info_, &connections_manager_,
kEndpointId);
session_.set_session_id(1234);
Expand All @@ -922,9 +922,12 @@ TEST_F(OutgoingShareSessionTest, DelayCompleteMetadataDisconnectTimeout) {
EXPECT_CALL(transfer_metadata_callback_,
Call(_, HasStatus(TransferMetadata::Status::kInProgress)));

session_.DelayCompleteMetadata(complete_metadata);
session_.DelayComplete(complete_metadata);
// Fast forward to the disconnection timeout.
fake_clock_.FastForward(absl::Seconds(60));
fake_task_runner_.SyncWithTimeout(absl::Milliseconds(100));

session_.DisconnectionTimeout();
// session_.DisconnectionTimeout();
// Verify that connection is closed.
EXPECT_THAT(
connections_manager_.connection_endpoint_info(kEndpointId).has_value(),
Expand Down

0 comments on commit 05a284c

Please sign in to comment.