Revert "tracing: clean up FD-passing when starting the service"

This reverts commit 2e6ece3.

Reason for revert: Caused an internal breakage in b/390202952.
Re-addressing in a different way.


Bug: 390202952
Change-Id: Icff7679a4dc77c42221c8f7493a4b00aa8417ee1

include/perfetto/ext/ipc/host.h

@@ -41,11 +41,14 @@ class Host {
  public:
   // Creates an instance and starts listening on the given |socket_name|.
   // Returns nullptr if listening on the socket fails.
-  // socket_name can be fd://123 for sockets pre-bound by init and passed as FD
-  // across exec in an env var.
   static std::unique_ptr<Host> CreateInstance(const char* socket_name,
                                               base::TaskRunner*);
 
+  // Like the above but takes a file descriptor to a pre-bound unix socket.
+  // Returns nullptr if listening on the socket fails.
+  static std::unique_ptr<Host> CreateInstance(base::ScopedSocketHandle,
+                                              base::TaskRunner*);
+
   // Creates a Host which is not backed by a POSIX listening socket.
   // Instead, it accepts sockets passed in via AdoptConnectedSocket_Fuchsia().
   // See go/fuchsetto for more details.

include/perfetto/ext/tracing/ipc/service_ipc_host.h

@@ -49,20 +49,22 @@ class PERFETTO_EXPORT_COMPONENT ServiceIPCHost {
 
   // The overload to wrap the multi-value producer socket name in the
   // single-value variant for compatibility in tests.
-  // The socket name can be fd://123 to pass a pre-bound socket. This is used
-  // when building as part of the Android tree, where init opens and binds the
-  // socket beore exec()-ing us.
   bool Start(const char* producer_socket_name,
              const char* consumer_socket_name) {
     return Start(TokenizeProducerSockets(producer_socket_name),
                  consumer_socket_name);
   }
-
   // Start listening on the Producer & Consumer ports. Returns false in case of
   // failure (e.g., something else is listening on |socket_name|).
   virtual bool Start(const std::vector<std::string>& producer_socket_names,
                      const char* consumer_socket_name) = 0;
 
+  // Like the above, but takes two file descriptors to already bound sockets.
+  // This is used when building as part of the Android tree, where init opens
+  // and binds the socket beore exec()-ing us.
+  virtual bool Start(base::ScopedSocketHandle producer_socket_fd,
+                     base::ScopedSocketHandle consumer_socket_fd) = 0;
+
   // Allows callers to supply preconstructed Hosts.
   virtual bool Start(std::unique_ptr<ipc::Host> producer_host,
                      std::unique_ptr<ipc::Host> consumer_host) = 0;

src/ipc/host_impl.cc

@@ -26,7 +26,6 @@
 #include "perfetto/base/task_runner.h"
 #include "perfetto/base/time.h"
 #include "perfetto/ext/base/crash_keys.h"
-#include "perfetto/ext/base/string_utils.h"
 #include "perfetto/ext/base/sys_types.h"
 #include "perfetto/ext/base/unix_socket.h"
 #include "perfetto/ext/base/utils.h"
@@ -124,34 +123,36 @@ std::unique_ptr<Host> Host::CreateInstance(const char* socket_name,
   return std::unique_ptr<Host>(std::move(host));
 }
 
+// static
+std::unique_ptr<Host> Host::CreateInstance(base::ScopedSocketHandle socket_fd,
+                                           base::TaskRunner* task_runner) {
+  std::unique_ptr<HostImpl> host(
+      new HostImpl(std::move(socket_fd), task_runner));
+  if (!host->sock() || !host->sock()->is_listening())
+    return nullptr;
+  return std::unique_ptr<Host>(std::move(host));
+}
+
 // static
 std::unique_ptr<Host> Host::CreateInstance_Fuchsia(
     base::TaskRunner* task_runner) {
   return std::unique_ptr<HostImpl>(new HostImpl(task_runner));
 }
 
-HostImpl::HostImpl(const char* socket_name, base::TaskRunner* task_runner)
+HostImpl::HostImpl(base::ScopedSocketHandle socket_fd,
+                   base::TaskRunner* task_runner)
     : task_runner_(task_runner), weak_ptr_factory_(this) {
   PERFETTO_DCHECK_THREAD(thread_checker_);
+  sock_ = base::UnixSocket::Listen(std::move(socket_fd), this, task_runner_,
+                                   kHostSockFamily, base::SockType::kStream);
+}
 
-  static constexpr char kFdPrefix[] = "fd://";
-  if (strncmp(socket_name, kFdPrefix, strlen(kFdPrefix)) == 0) {
-#if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
-    PERFETTO_FATAL("%s is only supported on POSIX systems", socket_name);
-#else
-    auto fd_num = base::CStringToInt32(&socket_name[strlen(kFdPrefix)]);
-    if (!fd_num.has_value()) {
-      PERFETTO_FATAL("Failed to parse fd:// number from %s", socket_name);
-    }
-    sock_ = base::UnixSocket::Listen(base::ScopedFile(*fd_num), this,
-                                     task_runner_, base::SockFamily::kUnix,
-                                     base::SockType::kStream);
-#endif
-  } else {
-    sock_ = base::UnixSocket::Listen(socket_name, this, task_runner_,
-                                     base::GetSockFamily(socket_name),
-                                     base::SockType::kStream);
-  }
+HostImpl::HostImpl(const char* socket_name, base::TaskRunner* task_runner)
+    : task_runner_(task_runner), weak_ptr_factory_(this) {
+  PERFETTO_DCHECK_THREAD(thread_checker_);
+  sock_ = base::UnixSocket::Listen(socket_name, this, task_runner_,
+                                   base::GetSockFamily(socket_name),
+                                   base::SockType::kStream);
   if (!sock_) {
     PERFETTO_PLOG("Failed to create %s", socket_name);
   }

src/ipc/host_impl.h

@@ -38,10 +38,9 @@ constexpr uint32_t kDefaultIpcTxTimeoutMs = 10000;
 
 class HostImpl : public Host, public base::UnixSocket::EventListener {
  public:
-  // socket_name can be fd://123 for sockets pre-bound by init and passed as FD
-  // across exec in an env var.
   HostImpl(const char* socket_name, base::TaskRunner*);
-  explicit HostImpl(base::TaskRunner* task_runner);
+  HostImpl(base::ScopedSocketHandle, base::TaskRunner*);
+  HostImpl(base::TaskRunner* task_runner);
   ~HostImpl() override;
 
   // Host implementation.

src/traced/service/service.cc

@@ -150,9 +150,9 @@ int PERFETTO_EXPORT_ENTRYPOINT ServiceMain(int argc, char** argv) {
 #if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
     PERFETTO_CHECK(false);
 #else
-    base::StackString<32> prod_sock("fd://%s", env_prod);
-    base::StackString<32> cons_sock("fd://%s", env_cons);
-    started = svc->Start({prod_sock.ToStdString()}, cons_sock.c_str());
+    base::ScopedFile producer_fd(atoi(env_prod));
+    base::ScopedFile consumer_fd(atoi(env_cons));
+    started = svc->Start(std::move(producer_fd), std::move(consumer_fd));
 #endif
   } else {
     auto producer_sockets = TokenizeProducerSockets(GetProducerSocket());

src/tracing/ipc/service/service_ipc_host_impl.cc

@@ -67,6 +67,18 @@ bool ServiceIPCHostImpl::Start(
   return DoStart();
 }
 
+bool ServiceIPCHostImpl::Start(base::ScopedSocketHandle producer_socket_fd,
+                               base::ScopedSocketHandle consumer_socket_fd) {
+  PERFETTO_CHECK(!svc_);  // Check if already started.
+
+  // Initialize the IPC transport.
+  producer_ipc_ports_.emplace_back(
+      ipc::Host::CreateInstance(std::move(producer_socket_fd), task_runner_));
+  consumer_ipc_port_ =
+      ipc::Host::CreateInstance(std::move(consumer_socket_fd), task_runner_);
+  return DoStart();
+}
+
 bool ServiceIPCHostImpl::Start(std::unique_ptr<ipc::Host> producer_host,
                                std::unique_ptr<ipc::Host> consumer_host) {
   PERFETTO_CHECK(!svc_);  // Check if already started.

src/tracing/ipc/service/service_ipc_host_impl.h

@@ -40,6 +40,8 @@ class ServiceIPCHostImpl : public ServiceIPCHost {
   // ServiceIPCHost implementation.
   bool Start(const std::vector<std::string>& producer_socket_names,
              const char* consumer_socket_name) override;
+  bool Start(base::ScopedSocketHandle producer_socket_fd,
+             base::ScopedSocketHandle consumer_socket_fd) override;
   bool Start(std::unique_ptr<ipc::Host> producer_host,
              std::unique_ptr<ipc::Host> consumer_host) override;