Merge "Fix base::Subprocess move ctor and operator="
diff --git a/include/perfetto/ext/ipc/client.h b/include/perfetto/ext/ipc/client.h
index 72c3e73..bf8242f 100644
--- a/include/perfetto/ext/ipc/client.h
+++ b/include/perfetto/ext/ipc/client.h
@@ -45,6 +45,7 @@
class Client {
public:
static std::unique_ptr<Client> CreateInstance(const char* socket_name,
+ bool socket_retry,
base::TaskRunner*);
virtual ~Client();
diff --git a/include/perfetto/ext/tracing/ipc/producer_ipc_client.h b/include/perfetto/ext/tracing/ipc/producer_ipc_client.h
index 5469ded..23a85d6 100644
--- a/include/perfetto/ext/tracing/ipc/producer_ipc_client.h
+++ b/include/perfetto/ext/tracing/ipc/producer_ipc_client.h
@@ -36,6 +36,16 @@
// src/tracing/ipc/producer/producer_ipc_client_impl.cc
class PERFETTO_EXPORT ProducerIPCClient {
public:
+ enum class ConnectionFlags {
+ // Fails immediately with OnConnect(false) if the service connection cannot
+ // be established.
+ kDefault = 0,
+
+ // Keeps retrying with exponential backoff indefinitely. The caller will
+ // never see an OnConnect(false).
+ kRetryIfUnreachable = 1,
+ };
+
// Connects to the producer port of the Service listening on the given
// |service_sock_name|. If the connection is successful, the OnConnect()
// method will be invoked asynchronously on the passed Producer interface. If
@@ -61,7 +71,8 @@
size_t shared_memory_size_hint_bytes = 0,
size_t shared_memory_page_size_hint_bytes = 0,
std::unique_ptr<SharedMemory> shm = nullptr,
- std::unique_ptr<SharedMemoryArbiter> shm_arbiter = nullptr);
+ std::unique_ptr<SharedMemoryArbiter> shm_arbiter = nullptr,
+ ConnectionFlags = ConnectionFlags::kDefault);
protected:
ProducerIPCClient() = delete;
diff --git a/src/ipc/client_impl.cc b/src/ipc/client_impl.cc
index c569eb4..f694547 100644
--- a/src/ipc/client_impl.cc
+++ b/src/ipc/client_impl.cc
@@ -38,16 +38,21 @@
// static
std::unique_ptr<Client> Client::CreateInstance(const char* socket_name,
+ bool socket_retry,
base::TaskRunner* task_runner) {
- std::unique_ptr<Client> client(new ClientImpl(socket_name, task_runner));
+ std::unique_ptr<Client> client(
+ new ClientImpl(socket_name, socket_retry, task_runner));
return client;
}
-ClientImpl::ClientImpl(const char* socket_name, base::TaskRunner* task_runner)
- : task_runner_(task_runner), weak_ptr_factory_(this) {
- sock_ = base::UnixSocket::Connect(socket_name, this, task_runner,
- base::SockFamily::kUnix,
- base::SockType::kStream);
+ClientImpl::ClientImpl(const char* socket_name,
+ bool socket_retry,
+ base::TaskRunner* task_runner)
+ : socket_name_(socket_name),
+ socket_retry_(socket_retry),
+ task_runner_(task_runner),
+ weak_ptr_factory_(this) {
+ TryConnect();
}
ClientImpl::~ClientImpl() {
@@ -57,6 +62,12 @@
nullptr); // The base::UnixSocket* ptr is not used in OnDisconnect().
}
+void ClientImpl::TryConnect() {
+ sock_ = base::UnixSocket::Connect(socket_name_, this, task_runner_,
+ base::SockFamily::kUnix,
+ base::SockType::kStream);
+}
+
void ClientImpl::BindService(base::WeakPtr<ServiceProxy> service_proxy) {
if (!service_proxy)
return;
@@ -129,6 +140,22 @@
}
void ClientImpl::OnConnect(base::UnixSocket*, bool connected) {
+ if (!connected && socket_retry_) {
+ socket_backoff_ms_ =
+ (socket_backoff_ms_ < 10000) ? socket_backoff_ms_ + 1000 : 30000;
+ PERFETTO_DLOG(
+ "Connection to traced's UNIX socket failed, retrying in %u seconds",
+ socket_backoff_ms_ / 1000);
+ auto weak_this = weak_ptr_factory_.GetWeakPtr();
+ task_runner_->PostDelayedTask(
+ [weak_this] {
+ if (weak_this)
+ static_cast<ClientImpl&>(*weak_this).TryConnect();
+ },
+ socket_backoff_ms_);
+ return;
+ }
+
// Drain the BindService() calls that were queued before establishig the
// connection with the host.
for (base::WeakPtr<ServiceProxy>& service_proxy : queued_bindings_) {
diff --git a/src/ipc/client_impl.h b/src/ipc/client_impl.h
index ff9c7bc..3947823 100644
--- a/src/ipc/client_impl.h
+++ b/src/ipc/client_impl.h
@@ -46,7 +46,7 @@
class ClientImpl : public Client, public base::UnixSocket::EventListener {
public:
- ClientImpl(const char* socket_name, base::TaskRunner*);
+ ClientImpl(const char* socket_name, bool socket_retry, base::TaskRunner*);
~ClientImpl() override;
// Client implementation.
@@ -81,6 +81,7 @@
ClientImpl(const ClientImpl&) = delete;
ClientImpl& operator=(const ClientImpl&) = delete;
+ void TryConnect();
bool SendFrame(const Frame&, int fd = -1);
void OnFrameReceived(const Frame&);
void OnBindServiceReply(QueuedRequest,
@@ -89,6 +90,9 @@
const protos::gen::IPCFrame_InvokeMethodReply&);
bool invoking_method_reply_ = false;
+ const char* socket_name_ = nullptr;
+ bool socket_retry_ = false;
+ uint32_t socket_backoff_ms_ = 0;
std::unique_ptr<base::UnixSocket> sock_;
base::TaskRunner* const task_runner_;
RequestID last_request_id_ = 0;
diff --git a/src/ipc/client_impl_unittest.cc b/src/ipc/client_impl_unittest.cc
index 183fbf0..61437af 100644
--- a/src/ipc/client_impl_unittest.cc
+++ b/src/ipc/client_impl_unittest.cc
@@ -205,7 +205,8 @@
void SetUp() override {
task_runner_.reset(new base::TestTaskRunner());
host_.reset(new FakeHost(task_runner_.get()));
- cli_ = Client::CreateInstance(kSockName, task_runner_.get());
+ cli_ =
+ Client::CreateInstance(kSockName, /*retry=*/false, task_runner_.get());
}
void TearDown() override {
diff --git a/src/ipc/test/ipc_integrationtest.cc b/src/ipc/test/ipc_integrationtest.cc
index b681f95..fe778b4 100644
--- a/src/ipc/test/ipc_integrationtest.cc
+++ b/src/ipc/test/ipc_integrationtest.cc
@@ -81,7 +81,7 @@
auto on_connect = task_runner_.CreateCheckpoint("on_connect");
EXPECT_CALL(svc_proxy_events_, OnConnect()).WillOnce(Invoke(on_connect));
std::unique_ptr<Client> cli =
- Client::CreateInstance(kSockName, &task_runner_);
+ Client::CreateInstance(kSockName, /*retry=*/false, &task_runner_);
std::unique_ptr<GreeterProxy> svc_proxy(new GreeterProxy(&svc_proxy_events_));
cli->BindService(svc_proxy->GetWeakPtr());
task_runner_.RunUntilCheckpoint("on_connect");
diff --git a/src/profiling/memory/BUILD.gn b/src/profiling/memory/BUILD.gn
index 5dd107c..4ad81ac 100644
--- a/src/profiling/memory/BUILD.gn
+++ b/src/profiling/memory/BUILD.gn
@@ -90,6 +90,15 @@
sources = [ "client_ext_standalone.cc" ]
}
+shared_library("heapprofd_standalone_client_noop") {
+ deps = [ "../../../gn:default_deps" ]
+ ldflags = [
+ "-Wl,--version-script",
+ rebase_path("heapprofd_client_api.map.txt", root_build_dir),
+ ]
+ sources = [ "client_ext_noop.cc" ]
+}
+
shared_library("heapprofd_client_api") {
configs -= [ "//gn/standalone:android_liblog" ]
if (perfetto_build_with_android) {
diff --git a/src/profiling/memory/client_ext_noop.cc b/src/profiling/memory/client_ext_noop.cc
new file mode 100644
index 0000000..1d2a98d
--- /dev/null
+++ b/src/profiling/memory/client_ext_noop.cc
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "perfetto/profiling/memory/client_ext.h"
+
+#include <inttypes.h>
+
+__attribute__((visibility("default"))) uint32_t heapprofd_register_heap(
+ const HeapprofdHeapInfo*,
+ size_t) {
+ return 0;
+}
+
+__attribute__((visibility("default"))) bool
+heapprofd_report_allocation(uint32_t, uint64_t, uint64_t) {
+ return false;
+}
+
+__attribute__((visibility("default"))) void heapprofd_report_free(uint32_t,
+ uint64_t) {}
+
+__attribute__((visibility("default"))) bool heapprofd_init_session(
+ void* (*)(size_t),
+ void (*)(void*)) {
+ return false;
+}
diff --git a/src/traced/probes/probes_producer.cc b/src/traced/probes/probes_producer.cc
index 69072c7..58389b8 100644
--- a/src/traced/probes/probes_producer.cc
+++ b/src/traced/probes/probes_producer.cc
@@ -129,7 +129,6 @@
// to reset all the data sources. Trying to handle that manually is going to
// be error prone. What we do here is simply destroying the instance and
// recreating it again.
- // TODO(hjd): Add e2e test for this.
base::TaskRunner* task_runner = task_runner_;
const char* socket_name = socket_name_;
diff --git a/src/tracing/internal/system_tracing_backend.cc b/src/tracing/internal/system_tracing_backend.cc
index 987f956..cbc53c8 100644
--- a/src/tracing/internal/system_tracing_backend.cc
+++ b/src/tracing/internal/system_tracing_backend.cc
@@ -40,7 +40,8 @@
auto endpoint = ProducerIPCClient::Connect(
GetProducerSocket(), args.producer, args.producer_name, args.task_runner,
TracingService::ProducerSMBScrapingMode::kEnabled,
- args.shmem_size_hint_bytes, args.shmem_page_size_hint_bytes);
+ args.shmem_size_hint_bytes, args.shmem_page_size_hint_bytes, nullptr,
+ nullptr, ProducerIPCClient::ConnectionFlags::kRetryIfUnreachable);
PERFETTO_CHECK(endpoint);
return endpoint;
}
diff --git a/src/tracing/internal/tracing_muxer_impl.cc b/src/tracing/internal/tracing_muxer_impl.cc
index 7511018..c0e6d4a 100644
--- a/src/tracing/internal/tracing_muxer_impl.cc
+++ b/src/tracing/internal/tracing_muxer_impl.cc
@@ -86,8 +86,15 @@
void TracingMuxerImpl::ProducerImpl::OnDisconnect() {
PERFETTO_DCHECK_THREAD(thread_checker_);
connected_ = false;
- // TODO: handle more graceful.
- PERFETTO_ELOG("Cannot connect to traced. Is it running?");
+ // TODO: handle more gracefully. Right now we only handle the case of retrying
+ // when not being able to reach the service in the first place (this is
+ // handled transparently by ProducerIPCClientImpl).
+ // If the connection is dropped afterwards (e.g., traced crashes), instead, we
+ // don't recover from that. In order to handle that we would have to reconnect
+ // and re-register all the data sources.
+ PERFETTO_ELOG(
+ "The connection to the tracing service dropped. Tracing will no longer "
+ "work until this process is restarted");
}
void TracingMuxerImpl::ProducerImpl::OnTracingSetup() {
diff --git a/src/tracing/ipc/consumer/consumer_ipc_client_impl.cc b/src/tracing/ipc/consumer/consumer_ipc_client_impl.cc
index 7d60e5e..336b2af 100644
--- a/src/tracing/ipc/consumer/consumer_ipc_client_impl.cc
+++ b/src/tracing/ipc/consumer/consumer_ipc_client_impl.cc
@@ -46,7 +46,9 @@
Consumer* consumer,
base::TaskRunner* task_runner)
: consumer_(consumer),
- ipc_channel_(ipc::Client::CreateInstance(service_sock_name, task_runner)),
+ ipc_channel_(ipc::Client::CreateInstance(service_sock_name,
+ /*retry=*/false,
+ task_runner)),
consumer_port_(this /* event_listener */),
weak_ptr_factory_(this) {
ipc_channel_->BindService(consumer_port_.GetWeakPtr());
diff --git a/src/tracing/ipc/producer/producer_ipc_client_impl.cc b/src/tracing/ipc/producer/producer_ipc_client_impl.cc
index 58fc05d..244d7df 100644
--- a/src/tracing/ipc/producer/producer_ipc_client_impl.cc
+++ b/src/tracing/ipc/producer/producer_ipc_client_impl.cc
@@ -47,13 +47,14 @@
size_t shared_memory_size_hint_bytes,
size_t shared_memory_page_size_hint_bytes,
std::unique_ptr<SharedMemory> shm,
- std::unique_ptr<SharedMemoryArbiter> shm_arbiter) {
+ std::unique_ptr<SharedMemoryArbiter> shm_arbiter,
+ ConnectionFlags conn_flags) {
return std::unique_ptr<TracingService::ProducerEndpoint>(
- new ProducerIPCClientImpl(service_sock_name, producer, producer_name,
- task_runner, smb_scraping_mode,
- shared_memory_size_hint_bytes,
- shared_memory_page_size_hint_bytes,
- std::move(shm), std::move(shm_arbiter)));
+ new ProducerIPCClientImpl(
+ service_sock_name, producer, producer_name, task_runner,
+ smb_scraping_mode, shared_memory_size_hint_bytes,
+ shared_memory_page_size_hint_bytes, std::move(shm),
+ std::move(shm_arbiter), conn_flags));
}
ProducerIPCClientImpl::ProducerIPCClientImpl(
@@ -65,10 +66,14 @@
size_t shared_memory_size_hint_bytes,
size_t shared_memory_page_size_hint_bytes,
std::unique_ptr<SharedMemory> shm,
- std::unique_ptr<SharedMemoryArbiter> shm_arbiter)
+ std::unique_ptr<SharedMemoryArbiter> shm_arbiter,
+ ProducerIPCClient::ConnectionFlags conn_flags)
: producer_(producer),
task_runner_(task_runner),
- ipc_channel_(ipc::Client::CreateInstance(service_sock_name, task_runner)),
+ ipc_channel_(ipc::Client::CreateInstance(
+ service_sock_name,
+ conn_flags == ProducerIPCClient::ConnectionFlags::kRetryIfUnreachable,
+ task_runner)),
producer_port_(this /* event_listener */),
shared_memory_(std::move(shm)),
shared_memory_arbiter_(std::move(shm_arbiter)),
diff --git a/src/tracing/ipc/producer/producer_ipc_client_impl.h b/src/tracing/ipc/producer/producer_ipc_client_impl.h
index 2995d38..64d9742 100644
--- a/src/tracing/ipc/producer/producer_ipc_client_impl.h
+++ b/src/tracing/ipc/producer/producer_ipc_client_impl.h
@@ -51,16 +51,16 @@
class ProducerIPCClientImpl : public TracingService::ProducerEndpoint,
public ipc::ServiceProxy::EventListener {
public:
- ProducerIPCClientImpl(
- const char* service_sock_name,
- Producer*,
- const std::string& producer_name,
- base::TaskRunner*,
- TracingService::ProducerSMBScrapingMode,
- size_t shared_memory_size_hint_bytes = 0,
- size_t shared_memory_page_size_hint_bytes = 0,
- std::unique_ptr<SharedMemory> shm = nullptr,
- std::unique_ptr<SharedMemoryArbiter> shm_arbiter = nullptr);
+ ProducerIPCClientImpl(const char* service_sock_name,
+ Producer*,
+ const std::string& producer_name,
+ base::TaskRunner*,
+ TracingService::ProducerSMBScrapingMode,
+ size_t shared_memory_size_hint_bytes,
+ size_t shared_memory_page_size_hint_bytes,
+ std::unique_ptr<SharedMemory> shm,
+ std::unique_ptr<SharedMemoryArbiter> shm_arbiter,
+ ProducerIPCClient::ConnectionFlags);
~ProducerIPCClientImpl() override;
// TracingService::ProducerEndpoint implementation.
diff --git a/tools/heap_profile b/tools/heap_profile
index a595e8c..812b8fa 100755
--- a/tools/heap_profile
+++ b/tools/heap_profile
@@ -89,11 +89,9 @@
config {{
name: "android.heapprofd"
heapprofd_config {{
-
shmem_size_bytes: {shmem_size}
sampling_interval_bytes: {interval}
{target_cfg}
-{continuous_dump_cfg}
}}
}}
}}
@@ -156,10 +154,10 @@
parser.add_argument(
"-d",
"--duration",
- help="Duration of profile (ms). "
- "Default 7 days.",
+ help="Duration of profile (ms). 0 to run until interrupted. "
+ "Default: until interrupted by user.",
type=int,
- default=604800000)
+ default=0)
# This flag is a no-op now. We never start heapprofd explicitly using system
# properties.
parser.add_argument(
@@ -296,21 +294,23 @@
target_cfg = ""
if not args.no_block_client:
- target_cfg += "block_client: true\n"
+ target_cfg += CFG_INDENT + "block_client: true\n"
if args.block_client_timeout:
- target_cfg += "block_client_timeout_us: %s\n" % args.block_client_timeout
+ target_cfg += (
+ CFG_INDENT + "block_client_timeout_us: %s\n" % args.block_client_timeout
+ )
if args.idle_allocations:
- target_cfg += "idle_allocations: true\n"
+ target_cfg += CFG_INDENT + "idle_allocations: true\n"
if args.no_startup:
- target_cfg += "no_startup: true\n"
+ target_cfg += CFG_INDENT + "no_startup: true\n"
if args.no_running:
- target_cfg += "no_running: true\n"
+ target_cfg += CFG_INDENT + "no_running: true\n"
if args.dump_at_max:
- target_cfg += "dump_at_max: true\n"
+ target_cfg += CFG_INDENT + "dump_at_max: true\n"
if args.disable_fork_teardown:
- target_cfg += "disable_fork_teardown: true\n"
+ target_cfg += CFG_INDENT + "disable_fork_teardown: true\n"
if args.all_heaps:
- target_cfg += "all_heaps: true\n"
+ target_cfg += CFG_INDENT + "all_heaps: true\n"
if args.pid:
for pid in args.pid.split(','):
try:
@@ -318,19 +318,36 @@
except ValueError:
print("FATAL: invalid PID %s" % pid, file=sys.stderr)
fail = True
- target_cfg += '{}pid: {}\n'.format(CFG_INDENT, pid)
+ target_cfg += CFG_INDENT + 'pid: {}\n'.format(pid)
if args.name:
for name in args.name.split(','):
- target_cfg += '{}process_cmdline: "{}"\n'.format(CFG_INDENT, name)
+ target_cfg += CFG_INDENT + 'process_cmdline: "{}"\n'.format(name)
if args.heaps:
for heap in args.heaps.split(','):
- target_cfg += '{}heaps: "{}"\n'.format(CFG_INDENT, heap)
+ target_cfg += CFG_INDENT + 'heaps: "{}"\n'.format(heap)
if fail:
parser.print_help()
return 1
trace_to_text_binary = args.trace_to_text_binary
+
+ if args.continuous_dump:
+ target_cfg += CONTINUOUS_DUMP.format(dump_interval=args.continuous_dump)
+ cfg = CFG.format(
+ interval=args.interval,
+ duration=args.duration,
+ target_cfg=target_cfg,
+ shmem_size=args.shmem_size)
+ if not args.no_versions:
+ cfg += PACKAGES_LIST_CFG
+
+ if args.print_config:
+ print(cfg)
+ return 0
+
+ # Do this AFTER print_config so we do not download trace_to_text only to
+ # print out the config.
if trace_to_text_binary is None:
platform = None
if sys.platform.startswith('linux'):
@@ -343,23 +360,6 @@
trace_to_text_binary = load_trace_to_text(platform)
- continuous_dump_cfg = ""
- if args.continuous_dump:
- continuous_dump_cfg = CONTINUOUS_DUMP.format(
- dump_interval=args.continuous_dump)
- cfg = CFG.format(
- interval=args.interval,
- duration=args.duration,
- target_cfg=target_cfg,
- continuous_dump_cfg=continuous_dump_cfg,
- shmem_size=args.shmem_size)
- if not args.no_versions:
- cfg += PACKAGES_LIST_CFG
-
- if args.print_config:
- print(cfg)
- return 0
-
# TODO(fmayer): Maybe feature detect whether we can remove traces instead of
# this.
uuid_trace = release_or_newer('R')