From fc37f9b9f037112da0bf00568ee50b6ff6d85044 Mon Sep 17 00:00:00 2001 From: Guogang Li Date: Thu, 7 Aug 2025 11:59:50 -0700 Subject: [PATCH] Internal bug fix PiperOrigin-RevId: 792253206 --- .../endpoint_channel_manager.cc | 62 +++++++++---------- .../Mediums/BLEv2/GNCBLEL2CAPConnection.m | 13 +++- .../apple/Mediums/BLEv2/GNCBLEL2CAPStream.m | 29 ++++++++- 3 files changed, 66 insertions(+), 38 deletions(-) diff --git a/connections/implementation/endpoint_channel_manager.cc b/connections/implementation/endpoint_channel_manager.cc index b88b757e..b093b6f3 100644 --- a/connections/implementation/endpoint_channel_manager.cc +++ b/connections/implementation/endpoint_channel_manager.cc @@ -22,6 +22,7 @@ #include "connections/implementation/client_proxy.h" #include "connections/implementation/endpoint_channel.h" #include "connections/implementation/offline_frames.h" +#include "connections/medium_selector.h" #include "internal/platform/condition_variable.h" #include "internal/platform/implementation/system_clock.h" #include "internal/platform/logging.h" @@ -49,7 +50,7 @@ void EndpointChannelManager::RegisterChannelForEndpoint( MutexLock lock(&mutex_); LOG(INFO) << "EndpointChannelManager registered channel of type " - << channel->GetType() << " to endpoint " << endpoint_id; + << channel->GetType() << " to endpoint " << endpoint_id; SetActiveEndpointChannel(client, endpoint_id, std::move(channel), true /* enable_encryption */); @@ -72,8 +73,8 @@ void EndpointChannelManager::ReplaceChannelForEndpoint( auto* endpoint = channel_state_.LookupEndpointData(endpoint_id); if (endpoint != nullptr && endpoint->channel == nullptr) { LOG(INFO) << "EndpointChannelManager is missing channel while " - "trying to update: endpoint " - << endpoint_id; + "trying to update: endpoint " + << endpoint_id; } SetActiveEndpointChannel(client, endpoint_id, std::move(channel), enable_encryption); @@ -128,11 +129,10 @@ bool EndpointChannelManager::isWifiLanConnected() const { } void EndpointChannelManager::UpdateSafeToDisconnectForEndpoint( - const std::string& endpoint_id, - bool safe_to_disconnect_enabled) { + const std::string& endpoint_id, bool safe_to_disconnect_enabled) { MutexLock lock(&mutex_); channel_state_.UpdateSafeToDisconnectForEndpoint(endpoint_id, - safe_to_disconnect_enabled); + safe_to_disconnect_enabled); } void EndpointChannelManager::MarkEndpointStopWaitToDisconnect( @@ -202,11 +202,10 @@ void EndpointChannelManager::ChannelState::UpdateEncryptionContextForEndpoint( } void EndpointChannelManager::ChannelState::UpdateSafeToDisconnectForEndpoint( - const std::string& endpoint_id, - bool safe_to_disconnect_enabled) { + const std::string& endpoint_id, bool safe_to_disconnect_enabled) { LOG(INFO) << "[safe-to-disconnect] " - "UpdateSafeToDisconnectForEndpoint for: " - << endpoint_id << " " << safe_to_disconnect_enabled; + "UpdateSafeToDisconnectForEndpoint for: " + << endpoint_id << " " << safe_to_disconnect_enabled; endpoints_[endpoint_id].safe_to_disconnect_enabled = safe_to_disconnect_enabled; @@ -217,7 +216,7 @@ bool EndpointChannelManager::ChannelState::GetSafeToDisconnectForEndpoint( auto item = endpoints_.find(endpoint_id); if (item == endpoints_.end()) return false; LOG(INFO) << "[safe-to-disconnect] GetSafeToDisconnectForEndpoint: " - << item->second.safe_to_disconnect_enabled; + << item->second.safe_to_disconnect_enabled; return item->second.safe_to_disconnect_enabled; } @@ -227,10 +226,9 @@ bool EndpointChannelManager::ChannelState::RemoveEndpoint( auto item = endpoints_.find(endpoint_id); if (item == endpoints_.end()) return false; - MarkEndpointStopWaitToDisconnect( - endpoint_id, - /* is_safe_to_disconnect */ true, - /* notify_stop_waiting */ true); + MarkEndpointStopWaitToDisconnect(endpoint_id, + /* is_safe_to_disconnect */ true, + /* notify_stop_waiting */ true); item->second.disconnect_reason = reason; auto channel = item->second.channel; @@ -240,7 +238,7 @@ bool EndpointChannelManager::ChannelState::RemoveEndpoint( channel->Resume(); LOG(INFO) << "[safe-to-disconnect] Sending DISCONNECTION frame" - " with request 0, ack 0"; + " with request 0, ack 0"; channel->Write( parser::ForDisconnection(/* request_safe_to_disconnect */ false, /* ack_safe_to_disconnect */ false)); @@ -260,8 +258,7 @@ bool EndpointChannelManager::ChannelState::isWifiLanConnected() const { auto channel = endpoint.second.channel; if (channel) { if (channel->GetMedium() == Medium::WIFI_LAN) { - LOG(INFO) << "Found WIFI_LAN Medium for endpoint:" - << endpoint.first; + LOG(INFO) << "Found WIFI_LAN Medium for endpoint:" << endpoint.first; return true; } } @@ -276,16 +273,16 @@ void EndpointChannelManager::ChannelState::MarkEndpointStopWaitToDisconnect( auto item = endpoints_.find(endpoint_id); if (item == endpoints_.end()) return; LOG(INFO) << "[safe-to-disconnect] is_safe_to_disconnect= " - << is_safe_to_disconnect - << ", notify_stop_waiting= " << notify_stop_waiting - << " for endpoint: " << endpoint_id; + << is_safe_to_disconnect + << ", notify_stop_waiting= " << notify_stop_waiting + << " for endpoint: " << endpoint_id; { MutexLock lock(&item->second.timeout_to_disconnected_mutex); item->second.is_safe_to_disconnect = is_safe_to_disconnect; if (!item->second.timeout_to_disconnected_enabled) return; if (notify_stop_waiting) { LOG(INFO) << "[safe-to-disconnect] Notify stop " - "waiting before timeout."; + "waiting before timeout."; item->second.timeout_to_disconnected.Notify(); item->second.timeout_to_disconnected_notified = true; } @@ -297,17 +294,16 @@ bool EndpointChannelManager::ChannelState::CreateNewTimeoutDisconnectedState( auto item = endpoints_.find(endpoint_id); if (item == endpoints_.end()) return false; LOG(INFO) << "[safe-to-disconnect] " - "Create TimeoutDisconnectedState for endpoint: " - << endpoint_id; + "Create TimeoutDisconnectedState for endpoint: " + << endpoint_id; { MutexLock lock(&item->second.timeout_to_disconnected_mutex); item->second.timeout_to_disconnected_enabled = true; item->second.timeout_to_disconnected_notified = false; item->second.timeout_to_disconnected.Wait(timeout_millis); LOG(INFO) << "[safe-to-disconnect] Wait is done with " - << (item->second.timeout_to_disconnected_notified - ? "notification" - : "timeout"); + << (item->second.timeout_to_disconnected_notified ? "notification" + : "timeout"); if (!item->second.timeout_to_disconnected_notified) item->second.is_safe_to_disconnect = true; item->second.timeout_to_disconnected_notified = false; @@ -322,16 +318,15 @@ bool EndpointChannelManager::ChannelState::IsWaitingForSafeToDisconnectTimeout( { MutexLock lock(&item->second.timeout_to_disconnected_mutex); LOG(INFO) << "[safe-to-disconnect] " - "IsWaitingForSafeToDisconnectTimeout for endpoint: " - << endpoint_id << ": " - << item->second.timeout_to_disconnected_enabled; + "IsWaitingForSafeToDisconnectTimeout for endpoint: " + << endpoint_id << ": " + << item->second.timeout_to_disconnected_enabled; return (item->second.timeout_to_disconnected_enabled); } } bool EndpointChannelManager::ChannelState::IsSafeToDisconnect( const std::string& endpoint_id) { - auto item = endpoints_.find(endpoint_id); if (item == endpoints_.end()) return true; { @@ -365,9 +360,8 @@ bool EndpointChannelManager::UnregisterChannelForEndpoint( safe_to_disconnect_enabled, result)) { return false; } - LOG(INFO) - << "EndpointChannelManager unregistered channel for endpoint " - << endpoint_id; + LOG(INFO) << "EndpointChannelManager unregistered channel for endpoint " + << endpoint_id; return true; } diff --git a/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEL2CAPConnection.m b/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEL2CAPConnection.m index 4f05435b..6664f0a4 100644 --- a/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEL2CAPConnection.m +++ b/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEL2CAPConnection.m @@ -172,7 +172,16 @@ static NSData *PrefixLengthData(NSData *data) { } - (void)stream:(GNCBLEL2CAPStream *)stream didDisconnectWithError:(NSError *_Nullable)error { + __weak __typeof__(self) weakSelf = self; + dispatch_async(_selfQueue, ^{ + __typeof__(self) strongSelf = weakSelf; + if (!strongSelf) { + return; + } + + [strongSelf->_stream close]; + if (_connectionHandlers.disconnectedHandler) { dispatch_async(_callbackQueue, ^{ _connectionHandlers.disconnectedHandler(); @@ -206,8 +215,8 @@ static NSData *PrefixLengthData(NSData *data) { } if (realData.length < _serviceIDHash.length) { - GNCLoggerError(@"Data length mismatch. Expected size: > %lu, Data: %@", - _serviceIDHash.length, realData); + GNCLoggerError(@"Data length mismatch. Expected size: > %lu, Data: %@", _serviceIDHash.length, + realData); return bytesProcessed; } diff --git a/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEL2CAPStream.m b/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEL2CAPStream.m index 018a50d0..81fb7c7e 100644 --- a/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEL2CAPStream.m +++ b/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEL2CAPStream.m @@ -16,6 +16,8 @@ #import "internal/platform/implementation/apple/Log/GNCLogger.h" +NS_ASSUME_NONNULL_BEGIN + enum { READ_BUFFER_SIZE = 409600 }; /** A pending packet that will be written to the L2CAP socket. */ @@ -69,6 +71,9 @@ enum { READ_BUFFER_SIZE = 409600 }; /// Verbose logging for some statements which are only useful when debugging but produce far too /// much log-spam to enable on Dev. BOOL _verboseLoggingEnabled; + + /// Whether the stream is closed. + BOOL _closed; } #pragma mark Public @@ -96,7 +101,13 @@ enum { READ_BUFFER_SIZE = 409600 }; } - (void)close { - _closedBlock(); + if (_closed) { + return; + } + if (_closedBlock) { + _closedBlock(); + } + _closed = YES; } - (void)tearDown { @@ -141,6 +152,12 @@ enum { READ_BUFFER_SIZE = 409600 }; GNCLoggerError(@"[NEARBY] Sending data cannot be nil or empty"); } + if (_closed) { + GNCLoggerInfo(@"[NEARBY] Sending data after stream is closed."); + completionBlock(YES); + return; + } + GNCBLEL2CAPStreamWriteOperation *write = [[GNCBLEL2CAPStreamWriteOperation alloc] initWithData:data completionBlock:completionBlock]; @synchronized(_writeBufferArray) { @@ -322,12 +339,18 @@ enum { READ_BUFFER_SIZE = 409600 }; }); } else if (bytesRead < 0) { GNCLoggerError(@"[NEARBY] Stream read error: %@", self.inputStream.streamError); + [_delegate stream:self didDisconnectWithError:self.inputStream.streamError]; + if (_closedBlock) { + _closedBlock(); + } } else if (bytesRead == 0) { GNCLoggerDebug(@"[NEARBY] End of stream reached. Disconnecting"); // This indicates the L2CAP socket is closed. Notifying the owner so that it can tear down this // stream and update its own state. [_delegate stream:self didDisconnectWithError:nil]; - _closedBlock(); + if (_closedBlock) { + _closedBlock(); + } } } @@ -350,3 +373,5 @@ enum { READ_BUFFER_SIZE = 409600 }; } @end + +NS_ASSUME_NONNULL_END