https://github.com/cs01 created https://github.com/llvm/llvm-project/pull/145382
AdbClient and PlatformAndroid GetSyncService is not threadsafe. This was not an
issue because it was (apparently) not used in a threaded environment. However
when the new setting
```
set target.parallel-module-load true
```
was added, it began being called from multiple threads to pull modules
simultaneously.
Setting it to True triggers crashes, False stops it from crashing.
The top of the stack in the crash looked something like this:
```
lldb_private::platform_android::AdbClient::SyncService::SendSyncRequest(char
const*, unsigned int, void const*) at /usr/lib/liblldb.so.15.0.0:0
lldb_private::platform_android::AdbClient::SyncService::internalStat(lldb_private::FileSpec
const&, unsigned int&, unsigned int&, unsigned int&) at
/usr/lib/liblldb.so.15.0.0:0
lldb_private::platform_android::AdbClient::SyncService::Stat(lldb_private::FileSpec
const&, unsigned int&, unsigned int&, unsigned int&) at
/usr/lib/liblldb.so.15.0.0:0
```
Our workaround was to set it to False, but the root cause was not fixed. This
PR fixes the root cause by creating a new connection for each Adb SyncService.
>From d585051b103acc1973671679bd1fa04b92fb0bf9 Mon Sep 17 00:00:00 2001
From: Chad Smith
Date: Mon, 23 Jun 2025 11:07:00 -0700
Subject: [PATCH] [lldb] make PlatformAndroid/AdbClient::GetSyncService
threadsafe
---
.../Plugins/Platform/Android/AdbClient.cpp| 29 -
.../Plugins/Platform/Android/AdbClient.h | 2 +
.../Platform/Android/PlatformAndroid.cpp | 10 +-
.../Platform/Android/PlatformAndroid.h| 4 +-
.../Platform/Android/AdbClientTest.cpp| 121 +-
5 files changed, 149 insertions(+), 17 deletions(-)
diff --git a/lldb/source/Plugins/Platform/Android/AdbClient.cpp
b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
index a179260ca15f6..4c5342f7af60b 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.cpp
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.cpp
@@ -417,13 +417,30 @@ Status AdbClient::ShellToFile(const char *command,
milliseconds timeout,
return Status();
}
+// Returns a sync service for file operations.
+// This operation is thread-safe - each call creates an isolated sync service
+// with its own connection to avoid race conditions.
std::unique_ptr
AdbClient::GetSyncService(Status &error) {
- std::unique_ptr sync_service;
- error = StartSync();
- if (error.Success())
-sync_service.reset(new SyncService(std::move(m_conn)));
+ std::lock_guard lock(m_sync_mutex);
+
+ // Create a temporary AdbClient with its own connection for this sync service
+ // This avoids the race condition of multiple threads accessing the same
+ // connection
+ AdbClient temp_client(m_device_id);
+
+ // Connect and start sync on the temporary client
+ error = temp_client.Connect();
+ if (error.Fail())
+return nullptr;
+ error = temp_client.StartSync();
+ if (error.Fail())
+return nullptr;
+
+ // Move the connection from the temporary client to the sync service
+ std::unique_ptr sync_service;
+ sync_service.reset(new SyncService(std::move(temp_client.m_conn)));
return sync_service;
}
@@ -487,7 +504,9 @@ Status AdbClient::SyncService::internalPushFile(const
FileSpec &local_file,
error.AsCString());
}
error = SendSyncRequest(
- kDONE,
llvm::sys::toTimeT(FileSystem::Instance().GetModificationTime(local_file)),
+ kDONE,
+ llvm::sys::toTimeT(
+ FileSystem::Instance().GetModificationTime(local_file)),
nullptr);
if (error.Fail())
return error;
diff --git a/lldb/source/Plugins/Platform/Android/AdbClient.h
b/lldb/source/Plugins/Platform/Android/AdbClient.h
index 851c09957bd4a..9ef780bc6202b 100644
--- a/lldb/source/Plugins/Platform/Android/AdbClient.h
+++ b/lldb/source/Plugins/Platform/Android/AdbClient.h
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
@@ -135,6 +136,7 @@ class AdbClient {
std::string m_device_id;
std::unique_ptr m_conn;
+ mutable std::mutex m_sync_mutex;
};
} // namespace platform_android
diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
index 5bc9cc133fbd3..68e4886cb1c7a 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -474,13 +474,11 @@ std::string PlatformAndroid::GetRunAs() {
return run_as.str();
}
-AdbClient::SyncService *PlatformAndroid::GetSyncService(Status &error) {
- if (m_adb_sync_svc && m_adb_sync_svc->IsConnected())
-return m_adb_sync_svc.get();
-
+std::unique_ptr
+PlatformAndroid::GetSyncService(Status &error) {
AdbClientUP adb(GetAdbClient(error));
if (error.Fail())
return nullptr;
- m_adb_sync_svc = adb->GetSyncService(error);
- return (error.Success()) ? m_adb_sync_svc.get() : nul
