Merge tag 'android-13.0.0_r52' into int/13/fp3
Android 13.0.0 Release 52 (TQ3A.230605.012)
* tag 'android-13.0.0_r52':
perfetto_cmd: --ignore-guardrails effects service
traced: bail out if an invalid trigger mode is specified
perfetto: fix enabling extra guardrails when reporting to fw
service: producer port: reduce blocking socket send timeout 10s -> 10ms
Populate property with the number of active OOME tracing sessions
Introduce guardrail override for max buffer size
profilers: treat platform UIDs [0..AID_APP_START) as profileable by the platform itself
Change-Id: I720f0cf504d6e08247ae724b5b8e80416862bed8
diff --git a/include/perfetto/ext/ipc/host.h b/include/perfetto/ext/ipc/host.h
index a9b5824..0f8533e 100644
--- a/include/perfetto/ext/ipc/host.h
+++ b/include/perfetto/ext/ipc/host.h
@@ -59,6 +59,9 @@
// case of errors (e.g., another service with the same name is already
// registered).
virtual bool ExposeService(std::unique_ptr<Service>) = 0;
+
+ // Overrides the default send timeout for the per-connection sockets.
+ virtual void SetSocketSendTimeoutMs(uint32_t timeout_ms) = 0;
};
} // namespace ipc
diff --git a/protos/perfetto/config/perfetto_config.proto b/protos/perfetto/config/perfetto_config.proto
index 00fee90..3bc7918 100644
--- a/protos/perfetto/config/perfetto_config.proto
+++ b/protos/perfetto/config/perfetto_config.proto
@@ -1918,6 +1918,10 @@
// On R-, this override only affected userdebug builds. Since S, it also
// affects user builds.
optional uint64 max_upload_per_day_bytes = 1;
+
+ // Overrides the guardrail for maximum trace buffer size.
+ // Available on U+
+ optional uint32 max_tracing_buffer_size_kb = 2;
}
optional GuardrailOverrides guardrail_overrides = 11;
diff --git a/protos/perfetto/config/trace_config.proto b/protos/perfetto/config/trace_config.proto
index be1ba82..4fc2717 100644
--- a/protos/perfetto/config/trace_config.proto
+++ b/protos/perfetto/config/trace_config.proto
@@ -208,6 +208,10 @@
// On R-, this override only affected userdebug builds. Since S, it also
// affects user builds.
optional uint64 max_upload_per_day_bytes = 1;
+
+ // Overrides the guardrail for maximum trace buffer size.
+ // Available on U+
+ optional uint32 max_tracing_buffer_size_kb = 2;
}
optional GuardrailOverrides guardrail_overrides = 11;
diff --git a/protos/perfetto/trace/perfetto_trace.proto b/protos/perfetto/trace/perfetto_trace.proto
index 58a0f68..7a80660 100644
--- a/protos/perfetto/trace/perfetto_trace.proto
+++ b/protos/perfetto/trace/perfetto_trace.proto
@@ -1918,6 +1918,10 @@
// On R-, this override only affected userdebug builds. Since S, it also
// affects user builds.
optional uint64 max_upload_per_day_bytes = 1;
+
+ // Overrides the guardrail for maximum trace buffer size.
+ // Available on U+
+ optional uint32 max_tracing_buffer_size_kb = 2;
}
optional GuardrailOverrides guardrail_overrides = 11;
diff --git a/src/android_stats/perfetto_atoms.h b/src/android_stats/perfetto_atoms.h
index 75cbbd8..14740ae 100644
--- a/src/android_stats/perfetto_atoms.h
+++ b/src/android_stats/perfetto_atoms.h
@@ -70,6 +70,7 @@
kTracedStartTracingInvalidSessionState = 36,
kTracedEnableTracingInvalidFilter = 47,
kTracedEnableTracingOobTargetBuffer = 48,
+ kTracedEnableTracingInvalidTriggerMode = 52,
// Checkpoints inside perfetto_cmd after tracing has finished.
kOnTracingDisabled = 4,
diff --git a/src/ipc/host_impl.cc b/src/ipc/host_impl.cc
index c182cd1..1bf2348 100644
--- a/src/ipc/host_impl.cc
+++ b/src/ipc/host_impl.cc
@@ -116,6 +116,12 @@
return true;
}
+void HostImpl::SetSocketSendTimeoutMs(uint32_t timeout_ms) {
+ PERFETTO_DCHECK_THREAD(thread_checker_);
+ // Should be less than the watchdog period (30s).
+ socket_tx_timeout_ms_ = timeout_ms;
+}
+
void HostImpl::OnNewIncomingConnection(
base::UnixSocket*,
std::unique_ptr<base::UnixSocket> new_conn) {
@@ -125,8 +131,7 @@
clients_by_socket_[new_conn.get()] = client.get();
client->id = client_id;
client->sock = std::move(new_conn);
- // Watchdog is 30 seconds, so set the socket timeout to 10 seconds.
- client->sock->SetTxTimeout(10000);
+ client->sock->SetTxTimeout(socket_tx_timeout_ms_);
clients_[client_id] = std::move(client);
}
diff --git a/src/ipc/host_impl.h b/src/ipc/host_impl.h
index ac0a593..de7d07e 100644
--- a/src/ipc/host_impl.h
+++ b/src/ipc/host_impl.h
@@ -32,6 +32,8 @@
namespace perfetto {
namespace ipc {
+constexpr uint32_t kDefaultIpcTxTimeoutMs = 10000;
+
class HostImpl : public Host, public base::UnixSocket::EventListener {
public:
HostImpl(const char* socket_name, base::TaskRunner*);
@@ -40,6 +42,7 @@
// Host implementation.
bool ExposeService(std::unique_ptr<Service>) override;
+ void SetSocketSendTimeoutMs(uint32_t timeout_ms) override;
// base::UnixSocket::EventListener implementation.
void OnNewIncomingConnection(base::UnixSocket*,
@@ -88,6 +91,7 @@
std::map<base::UnixSocket*, ClientConnection*> clients_by_socket_;
ServiceID last_service_id_ = 0;
ClientID last_client_id_ = 0;
+ uint32_t socket_tx_timeout_ms_ = kDefaultIpcTxTimeoutMs;
PERFETTO_THREAD_CHECKER(thread_checker_)
base::WeakPtrFactory<HostImpl> weak_ptr_factory_; // Keep last.
};
diff --git a/src/perfetto_cmd/perfetto_cmd.cc b/src/perfetto_cmd/perfetto_cmd.cc
index 3ed3140..aafc844 100644
--- a/src/perfetto_cmd/perfetto_cmd.cc
+++ b/src/perfetto_cmd/perfetto_cmd.cc
@@ -1002,7 +1002,9 @@
}
PERFETTO_DCHECK(trace_config_);
- trace_config_->set_enable_extra_guardrails(save_to_incidentd_);
+ trace_config_->set_enable_extra_guardrails(
+ (save_to_incidentd_ || report_to_android_framework_) &&
+ !ignore_guardrails_);
// Set the statsd logging flag if we're uploading
diff --git a/src/profiling/common/producer_support.cc b/src/profiling/common/producer_support.cc
index 8ab0b42..a160149 100644
--- a/src/profiling/common/producer_support.cc
+++ b/src/profiling/common/producer_support.cc
@@ -110,10 +110,25 @@
return true;
}
+ bool trusted_initiator = ds_config.session_initiator() ==
+ DataSourceConfig::SESSION_INITIATOR_TRUSTED_SYSTEM;
+
uint64_t uid_without_profile = uid % kAidUserOffset;
uint64_t uid_for_lookup = 0;
- if (uid_without_profile >= kAidAppStart &&
- uid_without_profile <= kAidAppEnd) {
+ if (uid_without_profile < kAidAppStart) {
+ // Platform processes are considered profileable by the platform itself.
+ // This includes platform UIDs from other profiles, e.g. "u10_system".
+ // It's possible that this is an app (e.g. com.android.settings runs as
+ // AID_SYSTEM), but we will skip checking packages.list for the profileable
+ // manifest flags, as running under a platform UID is considered sufficient.
+ // Minor consequence: shell cannot profile platform apps, even if their
+ // manifest flags opt into profiling from shell. Resolving this would
+ // require definitively disambiguating native processes from apps if both
+ // can run as the same platform UID.
+ return trusted_initiator;
+
+ } else if (uid_without_profile >= kAidAppStart &&
+ uid_without_profile <= kAidAppEnd) {
// normal app
uid_for_lookup = uid_without_profile;
@@ -133,8 +148,6 @@
// even be the package in which the service was defined).
// TODO(rsavitski): find a way for the platform to tell native services
// about isolated<->app relations.
- bool trusted_initiator = ds_config.session_initiator() ==
- DataSourceConfig::SESSION_INITIATOR_TRUSTED_SYSTEM;
return trusted_initiator &&
AllPackagesProfileableByTrustedInitiator(packages_list_path);
diff --git a/src/traced/service/builtin_producer.cc b/src/traced/service/builtin_producer.cc
index a2007be..0664b5d 100644
--- a/src/traced/service/builtin_producer.cc
+++ b/src/traced/service/builtin_producer.cc
@@ -46,9 +46,11 @@
constexpr char kHeapprofdDataSourceName[] = "android.heapprofd";
constexpr char kJavaHprofDataSourceName[] = "android.java_hprof";
+constexpr char kJavaHprofOomDataSourceName[] = "android.java_hprof.oom";
constexpr char kTracedPerfDataSourceName[] = "linux.perf";
constexpr char kLazyHeapprofdPropertyName[] = "traced.lazy.heapprofd";
constexpr char kLazyTracedPerfPropertyName[] = "traced.lazy.traced_perf";
+constexpr char kJavaHprofOomActivePropertyName[] = "traced.oome_heap_session.count";
} // namespace
@@ -64,6 +66,8 @@
SetAndroidProperty(kLazyHeapprofdPropertyName, "");
if (!lazy_traced_perf_.instance_ids.empty())
SetAndroidProperty(kLazyTracedPerfPropertyName, "");
+ if (!java_hprof_oome_instances_.empty())
+ SetAndroidProperty(kJavaHprofOomActivePropertyName, "");
}
void BuiltinProducer::ConnectInProcess(TracingService* svc) {
@@ -102,6 +106,11 @@
lazy_traced_perf_dsd.set_name(kTracedPerfDataSourceName);
endpoint_->RegisterDataSource(lazy_traced_perf_dsd);
}
+ {
+ DataSourceDescriptor java_hprof_oome_dsd;
+ java_hprof_oome_dsd.set_name(kJavaHprofOomDataSourceName);
+ endpoint_->RegisterDataSource(java_hprof_oome_dsd);
+ }
}
void BuiltinProducer::SetupDataSource(DataSourceInstanceID ds_id,
@@ -120,6 +129,13 @@
lazy_traced_perf_.instance_ids.emplace(ds_id);
return;
}
+
+ if (ds_config.name() == kJavaHprofOomDataSourceName) {
+ java_hprof_oome_instances_.emplace(ds_id);
+ SetAndroidProperty(kJavaHprofOomActivePropertyName,
+ std::to_string(java_hprof_oome_instances_.size()));
+ return;
+ }
}
void BuiltinProducer::StartDataSource(DataSourceInstanceID ds_id,
@@ -152,6 +168,13 @@
MaybeInitiateLazyStop(ds_id, &lazy_heapprofd_, kLazyHeapprofdPropertyName);
MaybeInitiateLazyStop(ds_id, &lazy_traced_perf_, kLazyTracedPerfPropertyName);
+
+ auto oome_it = java_hprof_oome_instances_.find(ds_id);
+ if (oome_it != java_hprof_oome_instances_.end()) {
+ java_hprof_oome_instances_.erase(oome_it);
+ SetAndroidProperty(kJavaHprofOomActivePropertyName,
+ std::to_string(java_hprof_oome_instances_.size()));
+ }
}
void BuiltinProducer::MaybeInitiateLazyStop(DataSourceInstanceID ds_id,
diff --git a/src/traced/service/builtin_producer.h b/src/traced/service/builtin_producer.h
index 9873458..7014d44 100644
--- a/src/traced/service/builtin_producer.h
+++ b/src/traced/service/builtin_producer.h
@@ -34,6 +34,7 @@
// * perfetto metatrace
// * lazy heapprofd daemon starter (android only)
// * lazy traced_perf daemon starter (android only)
+// * java_hprof oom data source counter (android only)
class BuiltinProducer : public Producer {
public:
BuiltinProducer(base::TaskRunner* task_runner, uint32_t lazy_stop_delay_ms);
@@ -83,6 +84,7 @@
MetatraceState metatrace_;
LazyAndroidDaemonState lazy_heapprofd_;
LazyAndroidDaemonState lazy_traced_perf_;
+ std::set<DataSourceInstanceID> java_hprof_oome_instances_;
base::WeakPtrFactory<BuiltinProducer> weak_factory_; // Keep last.
};
diff --git a/src/tracing/core/tracing_service_impl.cc b/src/tracing/core/tracing_service_impl.cc
index 807b955..9341973 100644
--- a/src/tracing/core/tracing_service_impl.cc
+++ b/src/tracing/core/tracing_service_impl.cc
@@ -613,6 +613,15 @@
cfg.trigger_config().trigger_timeout_ms());
}
+ // This check has been introduced in May 2023 after finding b/274931668.
+ if (static_cast<int>(cfg.trigger_config().trigger_mode()) >
+ TraceConfig::TriggerConfig::TriggerMode_MAX) {
+ MaybeLogUploadEvent(
+ cfg, PerfettoStatsdAtom::kTracedEnableTracingInvalidTriggerMode);
+ return PERFETTO_SVC_ERR(
+ "The trace config specified an invalid trigger_mode");
+ }
+
if (has_trigger_config && cfg.duration_ms() != 0) {
MaybeLogUploadEvent(
cfg, PerfettoStatsdAtom::kTracedEnableTracingDurationWithTrigger);
@@ -664,12 +673,16 @@
}
buf_size_sum += buf.size_kb();
}
- if (buf_size_sum > kGuardrailsMaxTracingBufferSizeKb) {
+
+ uint32_t max_tracing_buffer_size_kb =
+ std::max(kGuardrailsMaxTracingBufferSizeKb,
+ cfg.guardrail_overrides().max_tracing_buffer_size_kb());
+ if (buf_size_sum > max_tracing_buffer_size_kb) {
MaybeLogUploadEvent(
cfg, PerfettoStatsdAtom::kTracedEnableTracingBufferSizeTooLarge);
return PERFETTO_SVC_ERR("Requested too large trace buffer (%" PRIu64
"kB > %" PRIu32 " kB)",
- buf_size_sum, kGuardrailsMaxTracingBufferSizeKb);
+ buf_size_sum, max_tracing_buffer_size_kb);
}
}
@@ -940,6 +953,9 @@
tracing_session->config.set_duration_ms(
cfg.trigger_config().trigger_timeout_ms());
break;
+
+ // The case of unknown modes (coming from future versions of the service)
+ // is handled few lines above (search for TriggerMode_MAX).
}
tracing_session->state = TracingSession::CONFIGURED;
diff --git a/src/tracing/core/tracing_service_impl_unittest.cc b/src/tracing/core/tracing_service_impl_unittest.cc
index 00b7a3a..bce99e7 100644
--- a/src/tracing/core/tracing_service_impl_unittest.cc
+++ b/src/tracing/core/tracing_service_impl_unittest.cc
@@ -425,6 +425,31 @@
EXPECT_THAT(consumer->ReadBuffers(), IsEmpty());
}
+// Regression test for b/274931668. An unkonwn trigger should not cause a trace
+// that runs indefinitely.
+TEST_F(TracingServiceImplTest, FailOnUnknownTrigger) {
+ std::unique_ptr<MockConsumer> consumer = CreateMockConsumer();
+ consumer->Connect(svc.get());
+
+ std::unique_ptr<MockProducer> producer = CreateMockProducer();
+ producer->Connect(svc.get(), "mock_producer");
+ producer->RegisterDataSource("ds_1");
+
+ TraceConfig trace_config;
+ trace_config.add_buffers()->set_size_kb(128);
+ trace_config.add_data_sources()->mutable_config()->set_name("ds_1");
+ auto* trigger_config = trace_config.mutable_trigger_config();
+ trigger_config->set_trigger_mode(
+ static_cast<TraceConfig::TriggerConfig::TriggerMode>(
+ TraceConfig::TriggerConfig::TriggerMode_MAX + 1));
+ auto* trigger = trigger_config->add_triggers();
+ trigger->set_name("trigger_from_the_future");
+ trigger_config->set_trigger_timeout_ms(1);
+
+ consumer->EnableTracing(trace_config);
+ consumer->WaitForTracingDisabled();
+}
+
// Creates a tracing session with a START_TRACING trigger and checks that
// the session is not started when the configured trigger producer is different
// than the producer that sent the trigger.
diff --git a/src/tracing/ipc/service/service_ipc_host_impl.cc b/src/tracing/ipc/service/service_ipc_host_impl.cc
index 5618b3c..3598bf5 100644
--- a/src/tracing/ipc/service/service_ipc_host_impl.cc
+++ b/src/tracing/ipc/service/service_ipc_host_impl.cc
@@ -31,6 +31,10 @@
namespace perfetto {
+namespace {
+constexpr uint32_t kProducerSocketTxTimeoutMs = 10;
+}
+
// TODO(fmayer): implement per-uid connection limit (b/69093705).
// Implements the publicly exposed factory method declared in
@@ -85,6 +89,16 @@
return false;
}
+ // Lower the timeout for blocking socket sends to producers as we shouldn't
+ // normally exhaust the kernel send buffer unless the producer is
+ // unresponsive. We'll drop the connection if the timeout is hit (see
+ // UnixSocket::Send). Context in b/236813972, b/193234818.
+ // Consumer port continues using the default timeout (10s) as there are
+ // generally fewer consumer processes, and they're better behaved. Also the
+ // consumer port ipcs might exhaust the send buffer under normal operation
+ // due to large messages such as ReadBuffersResponse.
+ producer_ipc_port_->SetSocketSendTimeoutMs(kProducerSocketTxTimeoutMs);
+
// TODO(fmayer): add a test that destroyes the ServiceIPCHostImpl soon after
// Start() and checks that no spurious callbacks are issued.
bool producer_service_exposed = producer_ipc_port_->ExposeService(