tracing: Log more user-friendly warning when tracing is uninitialized
If tracing is uninitialized and the user tries to use it, log a more
helpful error message instead of crashing on a null pointer dereference
somewhere deep in the the codebase.
To avoid introducing null checks to hot code paths, we instead introduce
a fake tracing muxer/platform which will explicitly fail all operations.
These fake objects will be replaced with real ones at initialization
time.
We also add a perfetto::Tracing::IsInitialized query to check if tracing
was initialized or not.
Note that data sources and TRACE_EVENT trace points will still silently
act as if tracing is disabled if the client library hasn't been
initialized. This is by design to avoid races during initialization
where it can be hard to guarantee no TRACE_EVENT is encountered before
Perfetto is initialized.
Change-Id: Iba1424f7040dda4d04da7b84752b1f4ad3b4d3b2
diff --git a/Android.bp b/Android.bp
index 221cbb3..cdecf37 100644
--- a/Android.bp
+++ b/Android.bp
@@ -8159,6 +8159,7 @@
"src/tracing/event_context.cc",
"src/tracing/interceptor.cc",
"src/tracing/internal/interceptor_trace_writer.cc",
+ "src/tracing/internal/tracing_muxer_fake.cc",
"src/tracing/internal/tracing_muxer_impl.cc",
"src/tracing/internal/track_event_internal.cc",
"src/tracing/platform.cc",
diff --git a/BUILD b/BUILD
index 2579427..0593c73 100644
--- a/BUILD
+++ b/BUILD
@@ -1566,6 +1566,8 @@
"src/tracing/event_context.cc",
"src/tracing/interceptor.cc",
"src/tracing/internal/interceptor_trace_writer.cc",
+ "src/tracing/internal/tracing_muxer_fake.cc",
+ "src/tracing/internal/tracing_muxer_fake.h",
"src/tracing/internal/tracing_muxer_impl.cc",
"src/tracing/internal/tracing_muxer_impl.h",
"src/tracing/internal/track_event_internal.cc",
diff --git a/CHANGELOG b/CHANGELOG
index 6f3dd02..2df294e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
Unreleased:
Tracing service and probes:
- *
+ * Print more helpful error messages if the client library is used without
+ having been initialized.
Trace Processor:
*
UI:
diff --git a/include/perfetto/base/compiler.h b/include/perfetto/base/compiler.h
index e114744..1b99f5d 100644
--- a/include/perfetto/base/compiler.h
+++ b/include/perfetto/base/compiler.h
@@ -130,6 +130,15 @@
#define PERFETTO_NO_THREAD_SAFETY_ANALYSIS
#endif
+// Avoid calling the exit-time destructor on an object with static lifetime.
+#if defined(__clang__) && __has_attribute(no_destroy)
+#define PERFETTO_HAS_NO_DESTROY() 1
+#define PERFETTO_NO_DESTROY __attribute__((no_destroy))
+#else
+#define PERFETTO_HAS_NO_DESTROY() 0
+#define PERFETTO_NO_DESTROY
+#endif
+
namespace perfetto {
namespace base {
diff --git a/include/perfetto/tracing/data_source.h b/include/perfetto/tracing/data_source.h
index bbfa132..ee6a331 100644
--- a/include/perfetto/tracing/data_source.h
+++ b/include/perfetto/tracing/data_source.h
@@ -411,8 +411,6 @@
new DataSourceType(constructor_args...));
};
auto* tracing_impl = internal::TracingMuxer::Get();
- if (!tracing_impl)
- return false;
return tracing_impl->RegisterDataSource(descriptor, factory,
&static_state_);
}
diff --git a/include/perfetto/tracing/interceptor.h b/include/perfetto/tracing/interceptor.h
index 60997b0..643b5a4 100644
--- a/include/perfetto/tracing/interceptor.h
+++ b/include/perfetto/tracing/interceptor.h
@@ -172,6 +172,7 @@
namespace internal {
class InterceptorTraceWriter;
class TracingMuxer;
+class TracingMuxerFake;
class TracingMuxerImpl;
} // namespace internal
@@ -213,6 +214,7 @@
private:
friend class internal::InterceptorTraceWriter;
friend class internal::TracingMuxer;
+ friend class internal::TracingMuxerFake;
friend class internal::TracingMuxerImpl;
friend MockTracingMuxer;
template <class T>
diff --git a/include/perfetto/tracing/tracing.h b/include/perfetto/tracing/tracing.h
index c63cfe3..b859eb1 100644
--- a/include/perfetto/tracing/tracing.h
+++ b/include/perfetto/tracing/tracing.h
@@ -169,6 +169,9 @@
InitializeInternal(args_copy);
}
+ // Checks if tracing has been initialized by calling |Initialize|.
+ static bool IsInitialized();
+
// Start a new tracing session using the given tracing backend. Use
// |kUnspecifiedBackend| to select an available backend automatically.
// For the moment this can be used only when initializing tracing in
diff --git a/src/tracing/BUILD.gn b/src/tracing/BUILD.gn
index b43466f..6dd2d51 100644
--- a/src/tracing/BUILD.gn
+++ b/src/tracing/BUILD.gn
@@ -104,6 +104,8 @@
"event_context.cc",
"interceptor.cc",
"internal/interceptor_trace_writer.cc",
+ "internal/tracing_muxer_fake.cc",
+ "internal/tracing_muxer_fake.h",
"internal/tracing_muxer_impl.cc",
"internal/tracing_muxer_impl.h",
"internal/track_event_internal.cc",
diff --git a/src/tracing/interceptor.cc b/src/tracing/interceptor.cc
index 21bd41f..30d2e3f 100644
--- a/src/tracing/interceptor.cc
+++ b/src/tracing/interceptor.cc
@@ -30,12 +30,6 @@
InterceptorBase::TLSFactory tls_factory,
InterceptorBase::TracePacketCallback on_trace_packet) {
auto* tracing_impl = internal::TracingMuxer::Get();
- PERFETTO_DCHECK(tracing_impl);
- if (!tracing_impl) {
- PERFETTO_ELOG(
- "Call Tracing::Initialize() before registering interceptors.");
- return;
- }
tracing_impl->RegisterInterceptor(descriptor, factory, tls_factory,
on_trace_packet);
}
diff --git a/src/tracing/internal/tracing_muxer_fake.cc b/src/tracing/internal/tracing_muxer_fake.cc
new file mode 100644
index 0000000..fd4e0b5
--- /dev/null
+++ b/src/tracing/internal/tracing_muxer_fake.cc
@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2021 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 "src/tracing/internal/tracing_muxer_fake.h"
+
+namespace perfetto {
+namespace internal {
+namespace {
+
+PERFETTO_NORETURN void FailUninitialized() {
+ PERFETTO_FATAL(
+ "Tracing not initialized. Call perfetto::Tracing::Initialize() first.");
+}
+
+} // namespace
+
+#if PERFETTO_HAS_NO_DESTROY()
+// static
+PERFETTO_NO_DESTROY TracingMuxerFake::FakePlatform
+ TracingMuxerFake::FakePlatform::instance{};
+// static
+PERFETTO_NO_DESTROY TracingMuxerFake TracingMuxerFake::instance{};
+#endif // PERFETTO_HAS_NO_DESTROY()
+
+TracingMuxerFake::FakePlatform::~FakePlatform() = default;
+
+Platform::ThreadLocalObject*
+TracingMuxerFake::FakePlatform::GetOrCreateThreadLocalObject() {
+ FailUninitialized();
+}
+
+std::unique_ptr<base::TaskRunner>
+TracingMuxerFake::FakePlatform::CreateTaskRunner(const CreateTaskRunnerArgs&) {
+ FailUninitialized();
+}
+
+std::string TracingMuxerFake::FakePlatform::GetCurrentProcessName() {
+ FailUninitialized();
+}
+
+bool TracingMuxerFake::RegisterDataSource(const DataSourceDescriptor&,
+ DataSourceFactory,
+ DataSourceStaticState*) {
+ FailUninitialized();
+}
+
+std::unique_ptr<TraceWriterBase> TracingMuxerFake::CreateTraceWriter(
+ DataSourceStaticState*,
+ uint32_t,
+ DataSourceState*,
+ BufferExhaustedPolicy) {
+ FailUninitialized();
+}
+
+void TracingMuxerFake::DestroyStoppedTraceWritersForCurrentThread() {
+ FailUninitialized();
+}
+
+void TracingMuxerFake::RegisterInterceptor(
+ const InterceptorDescriptor&,
+ InterceptorFactory,
+ InterceptorBase::TLSFactory,
+ InterceptorBase::TracePacketCallback) {
+ FailUninitialized();
+}
+
+} // namespace internal
+} // namespace perfetto
diff --git a/src/tracing/internal/tracing_muxer_fake.h b/src/tracing/internal/tracing_muxer_fake.h
new file mode 100644
index 0000000..0c2902b
--- /dev/null
+++ b/src/tracing/internal/tracing_muxer_fake.h
@@ -0,0 +1,75 @@
+/*
+ * Copyright (C) 2021 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.
+ */
+
+#ifndef SRC_TRACING_INTERNAL_TRACING_MUXER_FAKE_H_
+#define SRC_TRACING_INTERNAL_TRACING_MUXER_FAKE_H_
+
+#include "perfetto/base/compiler.h"
+#include "perfetto/tracing/internal/tracing_muxer.h"
+
+namespace perfetto {
+namespace internal {
+
+// An always-fail implementation of TracingMuxer. Before tracing has been
+// initialiazed, all muxer operations will route here and fail with a helpful
+// error message. This is to avoid introducing null checks in
+// performance-critical parts of the codebase.
+class TracingMuxerFake : public TracingMuxer {
+ class FakePlatform : public Platform {
+ public:
+ ~FakePlatform() override;
+ ThreadLocalObject* GetOrCreateThreadLocalObject() override;
+ std::unique_ptr<base::TaskRunner> CreateTaskRunner(
+ const CreateTaskRunnerArgs&) override;
+ std::string GetCurrentProcessName() override;
+
+ static FakePlatform instance;
+ };
+
+ public:
+ TracingMuxerFake() : TracingMuxer(&FakePlatform::instance) {}
+
+ static constexpr TracingMuxerFake* Get() {
+#if PERFETTO_HAS_NO_DESTROY()
+ return &instance;
+#else
+ return nullptr;
+#endif
+ }
+
+ // TracingMuxer implementation.
+ bool RegisterDataSource(const DataSourceDescriptor&,
+ DataSourceFactory,
+ DataSourceStaticState*) override;
+ std::unique_ptr<TraceWriterBase> CreateTraceWriter(
+ DataSourceStaticState*,
+ uint32_t data_source_instance_index,
+ DataSourceState*,
+ BufferExhaustedPolicy buffer_exhausted_policy) override;
+ void DestroyStoppedTraceWritersForCurrentThread() override;
+ void RegisterInterceptor(const InterceptorDescriptor&,
+ InterceptorFactory,
+ InterceptorBase::TLSFactory,
+ InterceptorBase::TracePacketCallback) override;
+
+ private:
+ static TracingMuxerFake instance;
+};
+
+} // namespace internal
+} // namespace perfetto
+
+#endif // SRC_TRACING_INTERNAL_TRACING_MUXER_FAKE_H_
diff --git a/src/tracing/internal/tracing_muxer_impl.cc b/src/tracing/internal/tracing_muxer_impl.cc
index 4130954..a78a3d5 100644
--- a/src/tracing/internal/tracing_muxer_impl.cc
+++ b/src/tracing/internal/tracing_muxer_impl.cc
@@ -44,6 +44,8 @@
#include "protos/perfetto/config/interceptor_config.gen.h"
+#include "src/tracing/internal/tracing_muxer_fake.h"
+
#if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
#include <io.h> // For dup()
#else
@@ -621,7 +623,7 @@
// ----- End of TracingMuxerImpl::TracingSessionImpl
// static
-TracingMuxer* TracingMuxer::instance_ = nullptr;
+TracingMuxer* TracingMuxer::instance_ = TracingMuxerFake::Get();
// This is called by perfetto::Tracing::Initialize().
// Can be called on any thread. Typically, but not necessarily, that will be
@@ -1464,7 +1466,7 @@
}
void TracingMuxerImpl::InitializeInstance(const TracingInitArgs& args) {
- if (instance_)
+ if (instance_ != TracingMuxerFake::Get())
PERFETTO_FATAL("Tracing already initialized");
instance_ = new TracingMuxerImpl(args);
}
diff --git a/src/tracing/test/api_integrationtest.cc b/src/tracing/test/api_integrationtest.cc
index b06340d..e9f45a9 100644
--- a/src/tracing/test/api_integrationtest.cc
+++ b/src/tracing/test/api_integrationtest.cc
@@ -364,6 +364,14 @@
if (!(supported_backends & backend))
GTEST_SKIP();
+ static bool was_initialized;
+ if (!was_initialized) {
+ EXPECT_FALSE(perfetto::Tracing::IsInitialized());
+ was_initialized = true;
+ } else {
+ EXPECT_TRUE(perfetto::Tracing::IsInitialized());
+ }
+
// Since the client API can only be initialized once per process, initialize
// both the in-process and system backends for every test here. The actual
// service to be used is chosen by the test parameter.
diff --git a/src/tracing/tracing.cc b/src/tracing/tracing.cc
index 09c0553..7c61fb8 100644
--- a/src/tracing/tracing.cc
+++ b/src/tracing/tracing.cc
@@ -25,12 +25,14 @@
#include "src/tracing/internal/tracing_muxer_impl.h"
namespace perfetto {
+namespace {
+bool g_was_initialized = false;
+}
// static
void Tracing::InitializeInternal(const TracingInitArgs& args) {
- static bool was_initialized = false;
static TracingInitArgs init_args;
- if (was_initialized) {
+ if (g_was_initialized) {
if (!(init_args == args)) {
PERFETTO_ELOG(
"Tracing::Initialize() called more than once with different args. "
@@ -44,10 +46,15 @@
PERFETTO_CHECK(args.dcheck_is_on_ == PERFETTO_DCHECK_IS_ON());
internal::TracingMuxerImpl::InitializeInstance(args);
internal::TrackRegistry::InitializeInstance();
- was_initialized = true;
+ g_was_initialized = true;
init_args = args;
}
+// static
+bool Tracing::IsInitialized() {
+ return g_was_initialized;
+}
+
// static
std::unique_ptr<TracingSession> Tracing::NewTrace(BackendType backend) {
return static_cast<internal::TracingMuxerImpl*>(internal::TracingMuxer::Get())