Make remote-android platform to use dynamic local tcp ports when forwarding device ports.

[ovyalov]

Summary

There is no guarantees that dynamically allocated device port will be free on host side - make remote-android platform to use dynamic local tcp ports when forwarding device ports.

[labath]

lgtm

source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp

166

It might be good to add a comment explaining why is this code in a loop (= to retry, if someone manages to snatch the port from under us), as I don't think it will be obvious when someone finds this in the future.

[ovyalov]

Added the comment.

[ovyalov]

Files:

/lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp
/lldb/trunk/source/Plugins/Platform/Android/AdbClient.h
/lldb/trunk/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
/lldb/trunk/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h
/lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
/lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h

Users:

ovyalov (Author)

http://reviews.llvm.org/rL246574

source/Plugins/Platform/Android/AdbClient.h

public:
 
     const std::string&
     GetDeviceID() const;
 
     Error
     GetDevices (DeviceIDList &device_list);
 
     Error
-    SetPortForwarding (const uint16_t port);
+    SetPortForwarding (const uint16_t local_port, const uint16_t remote_port);
 
     Error
-    DeletePortForwarding (const uint16_t port);
+    DeletePortForwarding (const uint16_t local_port);
 
     Error
     PullFile (const FileSpec &remote_file, const FileSpec &local_file);
 
     Error
     PushFile (const FileSpec &local_file, const FileSpec &remote_file);
 
     Error

source/Plugins/Platform/Android/AdbClient.cpp

AdbClient::GetDevices (DeviceIDList &device_list)
 
     for (const auto device: devices)
         device_list.push_back (device.split ('\t').first);
 
     return error;
 }
 
 Error
-AdbClient::SetPortForwarding (const uint16_t port)
+AdbClient::SetPortForwarding (const uint16_t local_port, const uint16_t remote_port)
 {
     char message[48];
-    snprintf (message, sizeof (message), "forward:tcp:%d;tcp:%d", port, port);
+    snprintf (message, sizeof (message), "forward:tcp:%d;tcp:%d", local_port, remote_port);
 
     const auto error = SendDeviceMessage (message);
     if (error.Fail ())
         return error;
 
     return ReadResponseStatus ();
 }
 
 Error
-AdbClient::DeletePortForwarding (const uint16_t port)
+AdbClient::DeletePortForwarding (const uint16_t local_port)
 {
     char message[32];
-    snprintf (message, sizeof (message), "killforward:tcp:%d", port);
+    snprintf (message, sizeof (message), "killforward:tcp:%d", local_port);
 
     const auto error = SendDeviceMessage (message);
     if (error.Fail ())
         return error;
 
     return ReadResponseStatus ();
 }
 

source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h

protected:
 
     bool
     KillSpawnedProcess (lldb::pid_t pid) override;
 
     void
     DeleteForwardPort (lldb::pid_t pid);
 
     std::string
-    MakeServerUrl(const char* scheme,
-                  const char* hostname,
-                  uint16_t port) override;
+    MakeUrl(const char* scheme,
+            uint16_t port,
+            const char* path) override;
+
+    Error
+    SetPortForwarding(const lldb::pid_t pid, const uint16_t remote_port, uint16_t &local_port);
 
 private:
     DISALLOW_COPY_AND_ASSIGN (PlatformAndroidRemoteGDBServer);
 
 };
 
 } // namespace platform_android
 } // namespace lldb_private
 
 #endif  // liblldb_PlatformAndroidRemoteGDBServer_h_

source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp

 //===-- PlatformAndroidRemoteGDBServer.cpp ----------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 // Other libraries and framework includes
 #include "lldb/Core/Error.h"
 #include "lldb/Core/Log.h"
+#include "lldb/Host/Socket.h"
 
 // Project includes
 #include "AdbClient.h"
 #include "PlatformAndroidRemoteGDBServer.h"
 #include "Utility/UriParser.h"
 
 #include <sstream>
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace platform_android;
 
 static const lldb::pid_t g_remote_platform_pid = 0; // Alias for the process id of lldb-platform
 
 static Error
-ForwardPortWithAdb (uint16_t port, std::string& device_id)
+ForwardPortWithAdb (const uint16_t local_port, const uint16_t remote_port, std::string& device_id)
 {
     Log *log(GetLogIfAllCategoriesSet (LIBLLDB_LOG_PLATFORM));
 
     AdbClient adb;
     auto error = AdbClient::CreateByDeviceID(device_id, adb);
     if (error.Fail ())
         return error;
 
     device_id = adb.GetDeviceID();
     if (log)
         log->Printf("Connected to Android device \"%s\"", device_id.c_str ());
 
-    return adb.SetPortForwarding(port);
+    return adb.SetPortForwarding(local_port, remote_port);
 }
 
 static Error
-DeleteForwardPortWithAdb (uint16_t port, const std::string& device_id)
+DeleteForwardPortWithAdb (uint16_t local_port, const std::string& device_id)
 {
     AdbClient adb (device_id);
-    return adb.DeletePortForwarding (port);
+    return adb.DeletePortForwarding (local_port);
+}
+
+static Error
+FindUnusedPort (uint16_t& port)
+{
+    Socket* socket = nullptr;
+    auto error = Socket::TcpListen ("localhost:0", false, socket, nullptr);
+    if (error.Success ())
+    {
+        port = socket->GetLocalPortNumber ();
+        delete socket;
+    }
+    return error;
 }
 
 PlatformAndroidRemoteGDBServer::PlatformAndroidRemoteGDBServer ()
 {
 }
 
 PlatformAndroidRemoteGDBServer::~PlatformAndroidRemoteGDBServer ()
 {
     for (const auto& it : m_port_forwards)
         DeleteForwardPortWithAdb(it.second, m_device_id);
 }
 
 uint16_t
 PlatformAndroidRemoteGDBServer::LaunchGDBserverAndGetPort (lldb::pid_t &pid)
 {
-    uint16_t port = m_gdb_client.LaunchGDBserverAndGetPort (pid, "127.0.0.1");
-    if (port == 0)
-        return port;
-
-    Error error = ForwardPortWithAdb(port, m_device_id);
-    if (error.Fail ())
-        return 0;
-
-    m_port_forwards[pid] = port;
-
-    return port;
+    uint16_t remote_port = m_gdb_client.LaunchGDBserverAndGetPort (pid, "127.0.0.1");
+    if (remote_port == 0)
+        return remote_port;
+
+    uint16_t local_port = 0;
+    auto error = SetPortForwarding (pid, remote_port, local_port);
+    return error.Success() ? local_port : 0;
 }
 
 bool
 PlatformAndroidRemoteGDBServer::KillSpawnedProcess (lldb::pid_t pid)
 {
     DeleteForwardPort (pid);
     return m_gdb_client.KillSpawnedProcess (pid);
 }
 
 Error
 PlatformAndroidRemoteGDBServer::ConnectRemote (Args& args)
 {
     m_device_id.clear();
 
     if (args.GetArgumentCount() != 1)
         return Error("\"platform connect\" takes a single argument: <connect-url>");
 
-    int port;
+    int remote_port;
     std::string scheme, host, path;
     const char *url = args.GetArgumentAtIndex (0);
     if (!url)
         return Error("URL is null.");
-    if (!UriParser::Parse (url, scheme, host, port, path))
+    if (!UriParser::Parse (url, scheme, host, remote_port, path))
         return Error("Invalid URL: %s", url);
     if (scheme == "adb")
         m_device_id = host;
 
-    Error error = ForwardPortWithAdb(port, m_device_id);
-    if (error.Fail())
+    uint16_t local_port = 0;
+    auto error = SetPortForwarding (g_remote_platform_pid, remote_port, local_port);
+    if (error.Fail ())
         return error;
 
-    m_port_forwards[g_remote_platform_pid] = port;
+    const std::string new_url = MakeUrl(
+        scheme.c_str(), host.c_str(), local_port, path.c_str());
+    args.ReplaceArgumentAtIndex (0, new_url.c_str ());
+
+    Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM));
+    if (log)
+        log->Printf("Rewritten URL: %s", new_url.c_str());
 
     error = PlatformRemoteGDBServer::ConnectRemote(args);
     if (error.Fail ())
         DeleteForwardPort (g_remote_platform_pid);
 
     return error;
 }
 
PlatformAndroidRemoteGDBServer::DeleteForwardPort (lldb::pid_t pid)
     if (error.Fail()) {
         if (log)
             log->Printf("Failed to delete port forwarding (pid=%" PRIu64 ", port=%d, device=%s): %s",
                          pid, port, m_device_id.c_str(), error.AsCString());
     }
     m_port_forwards.erase(it);
 }
 
+Error
+PlatformAndroidRemoteGDBServer::SetPortForwarding(const lldb::pid_t pid,
+                                                  const uint16_t remote_port,
+                                                  uint16_t &local_port)
+{
+    static const int kAttempsNum = 5;
+
+    Error error;
+    // There is a race possibility that somebody will occupy
+    // a port while we're in between FindUnusedPort and ForwardPortWithAdb -
+    // adding the loop to mitigate such problem.
+    for (auto i = 0; i < kAttempsNum; ++i)
+    {
+        error = FindUnusedPort(local_port);
+        if (error.Fail())
+            return error;
+
+        error = ForwardPortWithAdb(local_port, remote_port, m_device_id);
+        if (error.Success())
+        {
+            m_port_forwards[pid] = local_port;
+            break;
+        }
+    }
+
+    return error;
+}
+
 std::string
-PlatformAndroidRemoteGDBServer::MakeServerUrl(const char* scheme,
-                                              const char* hostname,
-                                              uint16_t port)
+PlatformAndroidRemoteGDBServer::MakeUrl(const char* scheme,
+                                        uint16_t port,
+                                        const char* path)
 {
     std::ostringstream hostname_str;
     if (!strcmp(scheme, "adb"))
         hostname_str << "[" << hostname << "]";
     else
         hostname_str << hostname;
 
-    return PlatformRemoteGDBServer::MakeServerUrl(scheme,
-                                                  hostname_str.str().c_str(),
-                                                  port);
+    return PlatformRemoteGDBServer::MakeUrl(scheme,
+                                            port,
+                                            path);
 }

source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h

protected:
     // after launching the lldb-gdbserver.
     virtual uint16_t
     LaunchGDBserverAndGetPort (lldb::pid_t &pid);
 
     virtual bool
     KillSpawnedProcess (lldb::pid_t pid);
 
     virtual std::string
-    MakeServerUrl(const char* scheme,
-                  const char* hostname,
-                  uint16_t port);
+    MakeUrl(const char* scheme,
+            uint16_t port,
+            const char* path);
 
 private:
     std::string
     MakeGdbServerUrl(const std::string &platform_scheme,
                      const std::string &platform_hostname,
                      uint16_t port);
 
     DISALLOW_COPY_AND_ASSIGN (PlatformRemoteGDBServer);
 
 };
 
 } // namespace platform_gdb_server
 } // namespace lldb_private
 
 #endif  // liblldb_PlatformRemoteGDBServer_h_

source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp

PlatformRemoteGDBServer::MakeGdbServerUrl(const std::string &platform_scheme,
                                           const std::string &platform_hostname,
                                           uint16_t port)
 {
     const char *override_scheme = getenv("LLDB_PLATFORM_REMOTE_GDB_SERVER_SCHEME");
     const char *override_hostname = getenv("LLDB_PLATFORM_REMOTE_GDB_SERVER_HOSTNAME");
     const char *port_offset_c_str = getenv("LLDB_PLATFORM_REMOTE_GDB_SERVER_PORT_OFFSET");
     int port_offset = port_offset_c_str ? ::atoi(port_offset_c_str) : 0;
 
-    return MakeServerUrl(override_scheme ? override_scheme : platform_scheme.c_str(),
-                         override_hostname ? override_hostname : platform_hostname.c_str(),
-                         port + port_offset);
+    return MakeUrl(override_scheme ? override_scheme : platform_scheme.c_str(),
+                   port + port_offset,
+                   nullptr);
 }
 
 std::string
-PlatformRemoteGDBServer::MakeServerUrl(const char* scheme,
+PlatformRemoteGDBServer::MakeUrl(const char* scheme,
                                        const char* hostname,
-                                       uint16_t port)
+                                       uint16_t port,
+                                       const char* path)
 {
     StreamString result;
     result.Printf("%s://%s:%u", scheme, hostname, port);
+    if (path)
+        result.Write(path, strlen(path));
     return result.GetString();
 }