Fix ADB client disconnect issues

[ovyalov]

Summary

• Disconnect SyncService in case of pull/push/stat request failure and make Android platform to establish new ADB sync connection - ADB sync service disconnects a socket if any inbound command fails.
• Establish new ADB connection for every shell command.

[tberghammer]

Looks good

[ovyalov]

Files:

/lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp
/lldb/trunk/source/Plugins/Platform/Android/AdbClient.h
/lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.cpp
/lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.h

Users:

ovyalov (Author)

http://reviews.llvm.org/rL274638

source/Plugins/Platform/Android/AdbClient.h

 //===-- AdbClient.h ---------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_AdbClient_h_
 #define liblldb_AdbClient_h_
 
 // C Includes
 
 // C++ Includes
 
+#include <functional>
 #include <list>
 #include <memory>
 #include <string>
 #include <vector>
 
 // Other libraries and framework includes
 // Project includes
 
public:
         PullFile (const FileSpec &remote_file, const FileSpec &local_file);
 
         Error
         PushFile (const FileSpec &local_file, const FileSpec &remote_file);
 
         Error
         Stat (const FileSpec &remote_file, uint32_t &mode, uint32_t &size, uint32_t &mtime);
 
+        bool
+        IsConnected () const;
+
     private:
         explicit SyncService (std::unique_ptr<Connection> &&conn);
 
         Error
         SendSyncRequest (const char *request_id, const uint32_t data_len, const void *data);
 
         Error
         ReadSyncHeader (std::string &response_id, uint32_t &data_len); 
 
         Error
         PullFileChunk (std::vector<char> &buffer, bool &eof);
 
         Error
         ReadAllBytes (void *buffer, size_t size);
 
+        Error
+        internalPullFile (const FileSpec &remote_file, const FileSpec &local_file);
 
-        const std::unique_ptr<Connection> m_conn;   
+        Error
+        internalPushFile (const FileSpec &local_file, const FileSpec &remote_file);
+
+        Error
+        internalStat (const FileSpec &remote_file, uint32_t &mode, uint32_t &size, uint32_t &mtime);
+
+        Error
+        executeCommand (const std::function<Error()> &cmd);
+
+        std::unique_ptr<Connection> m_conn;   
     };
 
     static Error
     CreateByDeviceID(const std::string &device_id, AdbClient &adb);
 
     AdbClient ();
     explicit AdbClient (const std::string &device_id);
 

 private:
     Error
     Connect ();
 
     void
     SetDeviceID (const std::string &device_id);
 
     Error
-    SendMessage (const std::string &packet);
+    SendMessage (const std::string &packet, const bool reconnect = true);
 
     Error
     SendDeviceMessage (const std::string &packet);
 
     Error
     ReadMessage (std::vector<char> &message);
 
     Error

source/Plugins/Platform/Android/AdbClient.cpp

AdbClient::DeletePortForwarding (const uint16_t local_port)
     const auto error = SendDeviceMessage (message);
     if (error.Fail ())
         return error;
 
     return ReadResponseStatus ();
 }
 
 Error
-AdbClient::SendMessage (const std::string &packet)
+AdbClient::SendMessage (const std::string &packet, const bool reconnect)
 {
     Error error;
-    if (!m_conn || !m_conn->IsConnected())
+    if (!m_conn || reconnect)
     {
         error = Connect ();
         if (error.Fail ())
             return error;
     }
 
     char length_buffer[5];
     snprintf (length_buffer, sizeof (length_buffer), "%04x", static_cast<int>(packet.size ()));
if (error.Fail ())
         return Error ("Sync failed: %s", error.AsCString ());
 
     return error;
 }
 
 Error
 AdbClient::Sync ()
 {
-    auto error = SendMessage ("sync:");
+    auto error = SendMessage ("sync:", false);
     if (error.Fail ())
         return error;
 
     return ReadResponseStatus ();
 }
 
 Error
 AdbClient::ReadAllBytes (void *buffer, size_t size)
 {
     return ::ReadAllBytes (*m_conn, buffer, size);
 }
 
 Error
 AdbClient::Shell (const char* command, uint32_t timeout_ms, std::string* output)
 {
+    auto error = SwitchDeviceTransport ();
+    if (error.Fail ())
+        return Error ("Failed to switch to device transport: %s", error.AsCString ());
+
     StreamString adb_command;
     adb_command.Printf("shell:%s", command);
-    auto error = SendMessage (adb_command.GetData());
+    error = SendMessage (adb_command.GetData(), false);
     if (error.Fail ())
         return error;
 
     error = ReadResponseStatus ();
     if (error.Fail ())
         return error;
 
     std::vector<char> in_buffer;
AdbClient::GetSyncService (Error &error)
     error = StartSync ();
     if (error.Success ())
         sync_service.reset (new SyncService(std::move(m_conn)));
 
     return sync_service;
 }
 
 Error
-AdbClient::SyncService::PullFile (const FileSpec &remote_file, const FileSpec &local_file)
+AdbClient::SyncService::internalPullFile (const FileSpec &remote_file, const FileSpec &local_file)
 {
     const auto local_file_path = local_file.GetPath ();
     llvm::FileRemover local_file_remover (local_file_path.c_str ());
 
     std::ofstream dst (local_file_path, std::ios::out | std::ios::binary);
     if (!dst.is_open ())
         return Error ("Unable to open local file %s", local_file_path.c_str());
 
while (!eof)
             dst.write (&chunk[0], chunk.size ());
     }
 
     local_file_remover.releaseFile ();
     return error;
 }
 
 Error
-AdbClient::SyncService::PushFile (const FileSpec &local_file, const FileSpec &remote_file)
+AdbClient::SyncService::internalPushFile (const FileSpec &local_file, const FileSpec &remote_file)
 {
     const auto local_file_path (local_file.GetPath ());
     std::ifstream src (local_file_path.c_str(), std::ios::in | std::ios::binary);
     if (!src.is_open ())
         return Error ("Unable to open local file %s", local_file_path.c_str());
 
     std::stringstream file_description;
     file_description << remote_file.GetPath(false).c_str() << "," << kDefaultMode;
AdbClient::SyncService::internalPushFile (const FileSpec &local_file, const FileSpec &remote_file)
     // If there was an error reading the source file, finish the adb file
     // transfer first so that adb isn't expecting any more data.
     if (src.bad())
         return Error ("Failed read on %s", local_file_path.c_str());
     return error;
 }
 
 Error
-AdbClient::SyncService::Stat (const FileSpec &remote_file, uint32_t &mode, uint32_t &size, uint32_t &mtime)
+AdbClient::SyncService::internalStat (const FileSpec &remote_file, uint32_t &mode, uint32_t &size, uint32_t &mtime)
 {
     const std::string remote_file_path (remote_file.GetPath (false));
     auto error = SendSyncRequest (kSTAT, remote_file_path.length (), remote_file_path.c_str ());
     if (error.Fail ())
         return Error ("Failed to send request: %s", error.AsCString ());
 
     static const size_t stat_len = strlen (kSTAT);
     static const size_t response_len = stat_len + (sizeof (uint32_t) * 3);
if (strncmp (command_str, kSTAT, stat_len))
         return Error ("Got invalid stat command: %s", command_str);
 
     mode = extractor.GetU32 (&offset);
     size = extractor.GetU32 (&offset);
     mtime = extractor.GetU32 (&offset);
     return Error ();
 }
 
+Error
+AdbClient::SyncService::PullFile (const FileSpec &remote_file, const FileSpec &local_file)
+{
+    return executeCommand ([this, &remote_file, &local_file]() {
+        return internalPullFile (remote_file, local_file);
+    });
+}
+
+Error
+AdbClient::SyncService::PushFile (const FileSpec &local_file, const FileSpec &remote_file)
+{
+    return executeCommand ([this, &local_file, &remote_file]() {
+        return internalPushFile (local_file, remote_file);
+    });
+}
+
+Error
+AdbClient::SyncService::Stat (const FileSpec &remote_file, uint32_t &mode, uint32_t &size, uint32_t &mtime)
+{
+    return executeCommand ([this, &remote_file, &mode, &size, &mtime]() {
+        return internalStat (remote_file, mode, size, mtime);
+    });
+}
+
+bool
+AdbClient::SyncService::IsConnected () const
+{
+    return m_conn && m_conn->IsConnected ();
+}
+
 AdbClient::SyncService::SyncService(std::unique_ptr<Connection> &&conn):
 m_conn(std::move(conn))
 {
 }
 
+Error
+AdbClient::SyncService::executeCommand (const std::function<Error()> &cmd)
+{
+    if (!m_conn)
+        return Error ("SyncService is disconnected");
+
+    const auto error = cmd ();
+    if (error.Fail ())
+        m_conn.reset ();
+
+    return error;
+}
+
 AdbClient::SyncService::~SyncService () {}
 
 Error
 AdbClient::SyncService::SendSyncRequest (const char *request_id, const uint32_t data_len, const void *data)
 {
     const DataBufferSP data_sp (new DataBufferHeap (kSyncPacketLen, 0));
     DataEncoder encoder (data_sp, eByteOrderLittle, sizeof (void*));
     auto offset = encoder.PutData (0, request_id, strlen(request_id));

source/Plugins/Platform/Android/PlatformAndroid.h

protected:
         Error
         DownloadSymbolFile (const lldb::ModuleSP& module_sp,
                             const FileSpec& dst_file_spec) override;
 
         const char*
         GetLibdlFunctionDeclarations() const override;
 
     private:
-        std::unique_ptr<AdbClient> m_adb;
+        AdbClient::SyncService* GetSyncService (Error &error);
+
         std::unique_ptr<AdbClient::SyncService> m_adb_sync_svc;
         std::string m_device_id;
         uint32_t m_sdk_version;
 
         DISALLOW_COPY_AND_ASSIGN (PlatformAndroid);
     };
 
 } // namespace platofor_android
 } // namespace lldb_private
 
 #endif // liblldb_PlatformAndroid_h_

source/Plugins/Platform/Android/PlatformAndroid.cpp

PlatformAndroid::ConnectRemote(Args& args)
     if (!UriParser::Parse(url, scheme, host, port, path))
         return Error("Invalid URL: %s", url);
     if (host != "localhost")
         m_device_id = host;
 
     auto error = PlatformLinux::ConnectRemote(args);
     if (error.Success())
     {
-        m_adb.reset(new AdbClient);
-        error = AdbClient::CreateByDeviceID(m_device_id, *m_adb);
+        AdbClient adb;
+        error = AdbClient::CreateByDeviceID(m_device_id, adb);
         if (error.Fail())
             return error;
 
-        m_device_id = m_adb->GetDeviceID();
-        m_adb_sync_svc = m_adb->GetSyncService(error);
-        if (error.Fail())
-            return error;
-
-        error = m_adb->SwitchDeviceTransport();
+        m_device_id = adb.GetDeviceID();
     }
     return error;
 }
 
 Error
 PlatformAndroid::GetFile (const FileSpec& source,
                           const FileSpec& destination)
 {
     if (IsHost() || !m_remote_platform_sp)
         return PlatformLinux::GetFile(source, destination);
 
     FileSpec source_spec (source.GetPath (false), false, FileSpec::ePathSyntaxPosix);
     if (source_spec.IsRelative())
         source_spec = GetRemoteWorkingDirectory ().CopyByAppendingPathComponent (source_spec.GetCString (false));
 
-    return m_adb_sync_svc->PullFile (source_spec, destination);
+    Error error;
+    auto sync_service = GetSyncService (error);
+    if (error.Fail ())
+        return error;
+
+    return sync_service->PullFile (source_spec, destination);
 }
 
 Error
 PlatformAndroid::PutFile (const FileSpec& source,
                           const FileSpec& destination,
                           uint32_t uid,
                           uint32_t gid)
 {
     if (IsHost() || !m_remote_platform_sp)
         return PlatformLinux::PutFile (source, destination, uid, gid);
 
     FileSpec destination_spec (destination.GetPath (false), false, FileSpec::ePathSyntaxPosix);
     if (destination_spec.IsRelative())
         destination_spec = GetRemoteWorkingDirectory ().CopyByAppendingPathComponent (destination_spec.GetCString (false));
 
     // TODO: Set correct uid and gid on remote file.
-    return m_adb_sync_svc->PushFile(source, destination_spec);
+    Error error;
+    auto sync_service = GetSyncService (error);
+    if (error.Fail ())
+        return error;
+    return sync_service->PushFile(source, destination_spec);
 }
 
 const char *
 PlatformAndroid::GetCacheHostname ()
 {
     return m_device_id.c_str ();
 }
 

 {
     if (!IsConnected())
         return 0;
 
     if (m_sdk_version != 0)
         return m_sdk_version;
 
     std::string version_string;
-    Error error = m_adb->Shell("getprop ro.build.version.sdk", 5000 /* ms */, &version_string);
+    AdbClient adb(m_device_id);
+    Error error = adb.Shell("getprop ro.build.version.sdk", 5000 /* ms */, &version_string);
     version_string = llvm::StringRef(version_string).trim().str();
 
     if (error.Fail() || version_string.empty())
     {
         Log* log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM);
         if (log)
             log->Printf("Get SDK version failed. (error: %s, output: %s)",
                         error.AsCString(), version_string.c_str());
PlatformAndroid::DownloadSymbolFile (const lldb::ModuleSP& module_sp,
     // Symbolizer isn't available before SDK version 23
     if (GetSdkVersion() < 23)
         return Error("Symbol file generation only supported on SDK 23+");
 
     // If we already have symtab then we don't have to try and generate one
     if (module_sp->GetSectionList()->FindSectionByName(ConstString(".symtab")) != nullptr)
         return Error("Symtab already available in the module");
 
+    AdbClient adb(m_device_id);
     std::string tmpdir;
-    Error error = m_adb->Shell("mktemp --directory --tmpdir /data/local/tmp", 5000 /* ms */, &tmpdir);
+    Error error = adb.Shell("mktemp --directory --tmpdir /data/local/tmp", 5000 /* ms */, &tmpdir);
     if (error.Fail() || tmpdir.empty())
         return Error("Failed to generate temporary directory on the device (%s)", error.AsCString());
     tmpdir = llvm::StringRef(tmpdir).trim().str();
 
     // Create file remover for the temporary directory created on the device
     std::unique_ptr<std::string, std::function<void(std::string*)>> tmpdir_remover(
         &tmpdir,
-        [this](std::string* s) {
+        [this, &adb](std::string* s) {
             StreamString command;
             command.Printf("rm -rf %s", s->c_str());
-            Error error = m_adb->Shell(command.GetData(), 5000 /* ms */, nullptr);
+            Error error = adb.Shell(command.GetData(), 5000 /* ms */, nullptr);
 
             Log *log(GetLogIfAllCategoriesSet (LIBLLDB_LOG_PLATFORM));
             if (error.Fail())
                 log->Printf("Failed to remove temp directory: %s", error.AsCString());
         }
     );
 
     FileSpec symfile_platform_filespec(tmpdir.c_str(), false);
     symfile_platform_filespec.AppendPathComponent("symbolized.oat");
 
     // Execute oatdump on the remote device to generate a file with symtab
     StreamString command;
     command.Printf("oatdump --symbolize=%s --output=%s",
                    module_sp->GetPlatformFileSpec().GetCString(false),
                    symfile_platform_filespec.GetCString(false));
-    error = m_adb->Shell(command.GetData(), 60000 /* ms */, nullptr);
+    error = adb.Shell(command.GetData(), 60000 /* ms */, nullptr);
     if (error.Fail())
         return Error("Oatdump failed: %s", error.AsCString());
 
     // Download the symbolfile from the remote device
     return GetFile(symfile_platform_filespec, dst_file_spec);
 }
 
 bool

 {
     return R"(
               extern "C" void* dlopen(const char*, int) asm("__dl_dlopen");
               extern "C" void* dlsym(void*, const char*) asm("__dl_dlsym");
               extern "C" int   dlclose(void*) asm("__dl_dlclose");
               extern "C" char* dlerror(void) asm("__dl_dlerror");
              )";
 }
+
+AdbClient::SyncService*
+PlatformAndroid::GetSyncService (Error &error)
+{
+    if (m_adb_sync_svc && m_adb_sync_svc->IsConnected ())
+        return m_adb_sync_svc.get ();
+
+    AdbClient adb (m_device_id);
+    m_adb_sync_svc = adb.GetSyncService (error);
+    return (error.Success ()) ? m_adb_sync_svc.get () : nullptr;
+}
+