diff --git a/.github/workflows/validate.yaml b/.github/workflows/validate.yaml index 60285ac5a8..4bf31896e9 100644 --- a/.github/workflows/validate.yaml +++ b/.github/workflows/validate.yaml @@ -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 diff --git a/sharing/nearby_sharing_service_impl.cc b/sharing/nearby_sharing_service_impl.cc index 89e50d4c17..efe1d65e08 100644 --- a/sharing/nearby_sharing_service_impl.cc +++ b/sharing/nearby_sharing_service_impl.cc @@ -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; @@ -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( - *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 @@ -3293,9 +3275,6 @@ bool NearbySharingServiceImpl::FindDuplicateInOutgoingShareTargets( std::optional 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 diff --git a/sharing/outgoing_share_session.cc b/sharing/outgoing_share_session.cc index 767b5bf328..d2288cf218 100644 --- a/sharing/outgoing_share_session.cc +++ b/sharing/outgoing_share_session.cc @@ -506,7 +506,7 @@ std::optional 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 " @@ -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( + 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( diff --git a/sharing/outgoing_share_session.h b/sharing/outgoing_share_session.h index 4b729bc8f3..59fe52094b 100644 --- a/sharing/outgoing_share_session.h +++ b/sharing/outgoing_share_session.h @@ -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, @@ -195,6 +197,8 @@ class OutgoingShareSession : public ShareSession { std::unique_ptr mutual_acceptance_timeout_; std::optional pending_complete_metadata_; absl::Time connection_start_time_; + // Timeout waiting for remote disconnect in order to complete transfer. + std::unique_ptr disconnection_timeout_; }; } // namespace nearby::sharing diff --git a/sharing/outgoing_share_session_test.cc b/sharing/outgoing_share_session_test.cc index 9caac39071..c31468bcac 100644 --- a/sharing/outgoing_share_session_test.cc +++ b/sharing/outgoing_share_session_test.cc @@ -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); @@ -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); @@ -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(),