Fix recursive lock acquiring in OnPayloadTransferUpdate()

PiperOrigin-RevId: 656123375
This commit is contained in:
Anay Wadhera
2024-07-25 15:53:20 -07:00
committed by Copybara-Service
parent 7c7b629582
commit 0be7045809
2 changed files with 52 additions and 18 deletions
+41 -17
View File
@@ -803,9 +803,34 @@ void NearbyConnectionsManagerImpl::ProcessUnknownFilePathsToDelete(
}
}
std::optional<
std::weak_ptr<NearbyConnectionsManagerImpl::PayloadStatusListener>>
NearbyConnectionsManagerImpl::GetStatusListenerForId(int64_t payload_id) const {
MutexLock lock(&mutex_);
auto listener_it = payload_status_listeners_.find(payload_id);
if (listener_it == payload_status_listeners_.end()) {
return std::nullopt;
}
return listener_it->second;
}
NearbyConnectionImpl* NearbyConnectionsManagerImpl::GetConnectionForId(
absl::string_view endpoint_id) const {
MutexLock lock(&mutex_);
auto connection_it = connections_.find(endpoint_id);
if (connection_it == connections_.end()) return nullptr;
return connection_it->second.get();
}
void NearbyConnectionsManagerImpl::RemoveStatusListenerForPayloadId(
int64_t payload_id) {
MutexLock lock(&mutex_);
payload_status_listeners_.erase(payload_id);
}
void NearbyConnectionsManagerImpl::OnPayloadTransferUpdate(
absl::string_view endpoint_id, const PayloadTransferUpdate& update) {
MutexLock lock(&mutex_);
NL_LOG(INFO) << "Received payload transfer update id=" << update.payload_id
<< ",status=" << PayloadStatusToString(update.status)
<< ",total=" << update.total_bytes
@@ -814,21 +839,22 @@ void NearbyConnectionsManagerImpl::OnPayloadTransferUpdate(
// If this is a payload we've registered for, then forward its status to
// the PayloadStatusListener if it still exists. We don't need to do
// anything more with the payload.
auto listener_it = payload_status_listeners_.find(update.payload_id);
if (listener_it != payload_status_listeners_.end()) {
std::weak_ptr<PayloadStatusListener> listener = listener_it->second;
std::optional<std::weak_ptr<PayloadStatusListener>> listener =
GetStatusListenerForId(update.payload_id);
if (listener.has_value()) {
switch (update.status) {
case PayloadStatus::kInProgress:
break;
case PayloadStatus::kSuccess:
case PayloadStatus::kCanceled:
case PayloadStatus::kFailure:
payload_status_listeners_.erase(update.payload_id);
RemoveStatusListenerForPayloadId(update.payload_id);
break;
}
// Note: The listener might be invalidated, for example, if it is shared
// with another payload in the same transfer.
if (auto status_listener = listener.lock()) {
if (auto status_listener = listener->lock()) {
status_listener->OnStatusUpdate(
std::make_unique<PayloadTransferUpdate>(update),
GetUpgradedMedium(endpoint_id));
@@ -839,27 +865,25 @@ void NearbyConnectionsManagerImpl::OnPayloadTransferUpdate(
// If this is an incoming payload that we have not registered for, then
// we'll treat it as a control frame (e.g. IntroductionFrame) and
// forward it to the associated NearbyConnection.
auto payload_it = incoming_payloads_.find(update.payload_id);
if (payload_it == incoming_payloads_.end()) return;
auto payload = GetIncomingPayload(update.payload_id);
if (payload == nullptr) return;
if (payload_it->second.content.type != PayloadContent::Type::kBytes) {
if (payload->content.type != PayloadContent::Type::kBytes) {
NL_LOG(WARNING) << "Received unknown payload of file type. Cancelling.";
nearby_connections_service_->CancelPayload(kServiceId, payload_it->first,
nearby_connections_service_->CancelPayload(kServiceId, payload->id,
[](Status status) {});
ProcessUnknownFilePathsToDelete(
update.status, payload_it->second.content.type,
payload_it->second.content.file_payload.file.path);
ProcessUnknownFilePathsToDelete(update.status, payload->content.type,
payload->content.file_payload.file.path);
return;
}
if (update.status != PayloadStatus::kSuccess) return;
auto connections_it = connections_.find(endpoint_id);
if (connections_it == connections_.end()) return;
NearbyConnectionImpl* connection = GetConnectionForId(endpoint_id);
if (connection == nullptr) return;
NL_LOG(INFO) << "Writing incoming byte message to NearbyConnection.";
connections_it->second->WriteMessage(
payload_it->second.content.bytes_payload.bytes);
connection->WriteMessage(payload->content.bytes_payload.bytes);
}
void NearbyConnectionsManagerImpl::Reset() {
+11 -1
View File
@@ -78,7 +78,8 @@ class NearbyConnectionsManagerImpl : public NearbyConnectionsManager {
void RegisterPayloadStatusListener(
int64_t payload_id,
std::weak_ptr<PayloadStatusListener> listener) override;
const Payload* GetIncomingPayload(int64_t payload_id) const override;
const Payload* GetIncomingPayload(int64_t payload_id) const override
ABSL_LOCKS_EXCLUDED(mutex_);
void Cancel(int64_t payload_id) override;
void ClearIncomingPayloads() override;
std::optional<std::vector<uint8_t>> GetRawAuthenticationToken(
@@ -128,6 +129,15 @@ class NearbyConnectionsManagerImpl : public NearbyConnectionsManager {
const std::filesystem::path& path);
absl::flat_hash_set<std::filesystem::path> GetUnknownFilePathsToDelete();
std::optional<std::weak_ptr<PayloadStatusListener>> GetStatusListenerForId(
int64_t payload_id) const ABSL_LOCKS_EXCLUDED(mutex_);
NearbyConnectionImpl* GetConnectionForId(absl::string_view endpoint_id) const
ABSL_LOCKS_EXCLUDED(mutex_);
void RemoveStatusListenerForPayloadId(int64_t payload_id)
ABSL_LOCKS_EXCLUDED(mutex_);
void Reset();
std::optional<Medium> GetUpgradedMedium(absl::string_view endpoint_id) const;