Add NullTraceWriter
Add a null implementation of TraceWriter. This simplifies error handling
for clients of SharedMemoryArbiter since they always get a valid
TraceWriter. NullTraceWriter is backed by a ScatteredStreamWriter
which in turn is backed by a ScatteredStreamWriterNullDelegate.
ScatteredStreamWriterNullDelegate always returns the same chunk of
memory.
Change-Id: I7390c7698f414bffeb2ca1df900893af536a5f2c
diff --git a/Android.bp b/Android.bp
index de7759d..b9c1996 100644
--- a/Android.bp
+++ b/Android.bp
@@ -61,6 +61,7 @@
"src/protozero/message.cc",
"src/protozero/message_handle.cc",
"src/protozero/proto_utils.cc",
+ "src/protozero/scattered_stream_null_delegate.cc",
"src/protozero/scattered_stream_writer.cc",
"src/traced/probes/filesystem/fs_mount.cc",
"src/traced/probes/filesystem/inode_file_data_source.cc",
@@ -77,6 +78,7 @@
"src/tracing/core/data_source_descriptor.cc",
"src/tracing/core/ftrace_config.cc",
"src/tracing/core/id_allocator.cc",
+ "src/tracing/core/null_trace_writer.cc",
"src/tracing/core/packet_stream_validator.cc",
"src/tracing/core/service_impl.cc",
"src/tracing/core/shared_memory_abi.cc",
@@ -162,6 +164,7 @@
"src/protozero/message.cc",
"src/protozero/message_handle.cc",
"src/protozero/proto_utils.cc",
+ "src/protozero/scattered_stream_null_delegate.cc",
"src/protozero/scattered_stream_writer.cc",
"src/tracing/core/chrome_config.cc",
"src/tracing/core/commit_data_request.cc",
@@ -169,6 +172,7 @@
"src/tracing/core/data_source_descriptor.cc",
"src/tracing/core/ftrace_config.cc",
"src/tracing/core/id_allocator.cc",
+ "src/tracing/core/null_trace_writer.cc",
"src/tracing/core/packet_stream_validator.cc",
"src/tracing/core/service_impl.cc",
"src/tracing/core/shared_memory_abi.cc",
@@ -284,7 +288,6 @@
"src/ftrace_reader/proto_translation_table.cc",
"src/ftrace_reader/test/cpu_reader_support.cc",
"src/ftrace_reader/test/scattered_stream_delegate_for_testing.cc",
- "src/ftrace_reader/test/scattered_stream_null_delegate.cc",
"src/ipc/buffered_frame_deserializer.cc",
"src/ipc/client_impl.cc",
"src/ipc/deferred.cc",
@@ -296,6 +299,7 @@
"src/protozero/message.cc",
"src/protozero/message_handle.cc",
"src/protozero/proto_utils.cc",
+ "src/protozero/scattered_stream_null_delegate.cc",
"src/protozero/scattered_stream_writer.cc",
"src/traced/probes/filesystem/fs_mount.cc",
"src/traced/probes/filesystem/inode_file_data_source.cc",
@@ -310,6 +314,7 @@
"src/tracing/core/data_source_descriptor.cc",
"src/tracing/core/ftrace_config.cc",
"src/tracing/core/id_allocator.cc",
+ "src/tracing/core/null_trace_writer.cc",
"src/tracing/core/packet_stream_validator.cc",
"src/tracing/core/service_impl.cc",
"src/tracing/core/shared_memory_abi.cc",
@@ -3119,6 +3124,7 @@
"src/protozero/message.cc",
"src/protozero/message_handle.cc",
"src/protozero/proto_utils.cc",
+ "src/protozero/scattered_stream_null_delegate.cc",
"src/protozero/scattered_stream_writer.cc",
"src/tracing/core/chrome_config.cc",
"src/tracing/core/commit_data_request.cc",
@@ -3126,6 +3132,7 @@
"src/tracing/core/data_source_descriptor.cc",
"src/tracing/core/ftrace_config.cc",
"src/tracing/core/id_allocator.cc",
+ "src/tracing/core/null_trace_writer.cc",
"src/tracing/core/packet_stream_validator.cc",
"src/tracing/core/service_impl.cc",
"src/tracing/core/shared_memory_abi.cc",
@@ -3305,7 +3312,6 @@
"src/ftrace_reader/proto_translation_table_unittest.cc",
"src/ftrace_reader/test/cpu_reader_support.cc",
"src/ftrace_reader/test/scattered_stream_delegate_for_testing.cc",
- "src/ftrace_reader/test/scattered_stream_null_delegate.cc",
"src/ipc/buffered_frame_deserializer.cc",
"src/ipc/buffered_frame_deserializer_unittest.cc",
"src/ipc/client_impl.cc",
@@ -3329,6 +3335,7 @@
"src/protozero/message_unittest.cc",
"src/protozero/proto_utils.cc",
"src/protozero/proto_utils_unittest.cc",
+ "src/protozero/scattered_stream_null_delegate.cc",
"src/protozero/scattered_stream_writer.cc",
"src/protozero/scattered_stream_writer_unittest.cc",
"src/protozero/test/fake_scattered_buffer.cc",
@@ -3352,6 +3359,8 @@
"src/tracing/core/ftrace_config.cc",
"src/tracing/core/id_allocator.cc",
"src/tracing/core/id_allocator_unittest.cc",
+ "src/tracing/core/null_trace_writer.cc",
+ "src/tracing/core/null_trace_writer_unittest.cc",
"src/tracing/core/packet_stream_validator.cc",
"src/tracing/core/packet_stream_validator_unittest.cc",
"src/tracing/core/patch_list_unittest.cc",
diff --git a/include/perfetto/protozero/BUILD.gn b/include/perfetto/protozero/BUILD.gn
index 7b91fe4..b97b137 100644
--- a/include/perfetto/protozero/BUILD.gn
+++ b/include/perfetto/protozero/BUILD.gn
@@ -21,6 +21,7 @@
"message.h",
"message_handle.h",
"proto_field_descriptor.h",
+ "scattered_stream_null_delegate.h",
"scattered_stream_writer.h",
]
}
diff --git a/src/ftrace_reader/test/scattered_stream_null_delegate.h b/include/perfetto/protozero/scattered_stream_null_delegate.h
similarity index 61%
rename from src/ftrace_reader/test/scattered_stream_null_delegate.h
rename to include/perfetto/protozero/scattered_stream_null_delegate.h
index 41e2bd8..8efb477 100644
--- a/src/ftrace_reader/test/scattered_stream_null_delegate.h
+++ b/include/perfetto/protozero/scattered_stream_null_delegate.h
@@ -14,31 +14,32 @@
* limitations under the License.
*/
-#ifndef SRC_FTRACE_READER_TEST_SCATTERED_STREAM_NULL_DELEGATE_H_
-#define SRC_FTRACE_READER_TEST_SCATTERED_STREAM_NULL_DELEGATE_H_
+#ifndef INCLUDE_PERFETTO_PROTOZERO_SCATTERED_STREAM_NULL_DELEGATE_H_
+#define INCLUDE_PERFETTO_PROTOZERO_SCATTERED_STREAM_NULL_DELEGATE_H_
#include <memory>
#include <vector>
#include "perfetto/base/logging.h"
+#include "perfetto/protozero/contiguous_memory_range.h"
#include "perfetto/protozero/scattered_stream_writer.h"
-namespace perfetto {
+namespace protozero {
-class ScatteredStreamNullDelegate
- : public protozero::ScatteredStreamWriter::Delegate {
+class ScatteredStreamWriterNullDelegate
+ : public ScatteredStreamWriter::Delegate {
public:
- explicit ScatteredStreamNullDelegate(size_t chunk_size);
- ~ScatteredStreamNullDelegate() override;
+ explicit ScatteredStreamWriterNullDelegate(size_t chunk_size);
+ ~ScatteredStreamWriterNullDelegate() override;
// protozero::ScatteredStreamWriter::Delegate implementation.
- protozero::ContiguousMemoryRange GetNewBuffer() override;
+ ContiguousMemoryRange GetNewBuffer() override;
private:
const size_t chunk_size_;
std::unique_ptr<uint8_t[]> chunk_;
};
-} // namespace perfetto
+} // namespace protozero
-#endif // SRC_FTRACE_READER_TEST_SCATTERED_STREAM_NULL_DELEGATE_H_
+#endif // INCLUDE_PERFETTO_PROTOZERO_SCATTERED_STREAM_NULL_DELEGATE_H_
diff --git a/include/perfetto/tracing/core/shared_memory_arbiter.h b/include/perfetto/tracing/core/shared_memory_arbiter.h
index 9efbba8..2cd336c 100644
--- a/include/perfetto/tracing/core/shared_memory_arbiter.h
+++ b/include/perfetto/tracing/core/shared_memory_arbiter.h
@@ -46,8 +46,7 @@
// Creates a new TraceWriter and assigns it a new WriterID. The WriterID is
// written in each chunk header owned by a given TraceWriter and is used by
// the Service to reconstruct TracePackets written by the same TraceWriter.
- // Returns nullptr if all WriterID slots are exhausted.
- // TODO(primiano): instead of nullptr this should return a NoopWriter.
+ // Returns null impl of TraceWriter if all WriterID slots are exhausted.
virtual std::unique_ptr<TraceWriter> CreateTraceWriter(
BufferID target_buffer) = 0;
diff --git a/src/ftrace_reader/BUILD.gn b/src/ftrace_reader/BUILD.gn
index 7e2a6a6..46de5df 100644
--- a/src/ftrace_reader/BUILD.gn
+++ b/src/ftrace_reader/BUILD.gn
@@ -35,8 +35,6 @@
"test/cpu_reader_support.h",
"test/scattered_stream_delegate_for_testing.cc",
"test/scattered_stream_delegate_for_testing.h",
- "test/scattered_stream_null_delegate.cc",
- "test/scattered_stream_null_delegate.h",
]
}
diff --git a/src/ftrace_reader/cpu_reader_benchmark.cc b/src/ftrace_reader/cpu_reader_benchmark.cc
index 8771bac..80ca016 100644
--- a/src/ftrace_reader/cpu_reader_benchmark.cc
+++ b/src/ftrace_reader/cpu_reader_benchmark.cc
@@ -16,16 +16,15 @@
#include "src/ftrace_reader/cpu_reader.h"
+#include "perfetto/base/utils.h"
+#include "perfetto/protozero/scattered_stream_null_delegate.h"
#include "perfetto/protozero/scattered_stream_writer.h"
#include "perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
#include "test/cpu_reader_support.h"
-#include "test/scattered_stream_null_delegate.h"
namespace {
-constexpr size_t kPageSize = 4096;
-
perfetto::ExamplePage g_full_page_sched_switch{
"synthetic",
R"(
@@ -293,7 +292,8 @@
using perfetto::ExamplePage;
using perfetto::EventFilter;
using perfetto::ProtoTranslationTable;
-using perfetto::ScatteredStreamNullDelegate;
+using protozero::ScatteredStreamWriterNullDelegate;
+using protozero::ScatteredStreamWriter;
using perfetto::GetTable;
using perfetto::PageFromXxd;
using perfetto::protos::pbzero::FtraceEventBundle;
@@ -303,8 +303,8 @@
static void BM_ParsePageFullOfSchedSwitch(benchmark::State& state) {
const ExamplePage* test_case = &g_full_page_sched_switch;
- ScatteredStreamNullDelegate delegate(kPageSize);
- protozero::ScatteredStreamWriter stream(&delegate);
+ ScatteredStreamWriterNullDelegate delegate(perfetto::base::kPageSize);
+ ScatteredStreamWriter stream(&delegate);
FtraceEventBundle writer;
ProtoTranslationTable* table = GetTable(test_case->name);
diff --git a/src/ftrace_reader/cpu_reader_fuzzer.cc b/src/ftrace_reader/cpu_reader_fuzzer.cc
index c41b528..e1ebae1 100644
--- a/src/ftrace_reader/cpu_reader_fuzzer.cc
+++ b/src/ftrace_reader/cpu_reader_fuzzer.cc
@@ -21,11 +21,11 @@
#include "perfetto/base/logging.h"
#include "perfetto/base/utils.h"
+#include "perfetto/protozero/scattered_stream_null_delegate.h"
#include "perfetto/protozero/scattered_stream_writer.h"
#include "perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
#include "src/ftrace_reader/cpu_reader.h"
#include "test/cpu_reader_support.h"
-#include "test/scattered_stream_null_delegate.h"
namespace perfetto {
namespace {
@@ -39,7 +39,7 @@
void FuzzCpuReaderParsePage(const uint8_t* data, size_t size);
void FuzzCpuReaderParsePage(const uint8_t* data, size_t size) {
- ScatteredStreamNullDelegate delegate(base::kPageSize);
+ protozero::ScatteredStreamWriterNullDelegate delegate(base::kPageSize);
protozero::ScatteredStreamWriter stream(&delegate);
FtraceEventBundle writer;
diff --git a/src/ftrace_reader/test/cpu_reader_support.cc b/src/ftrace_reader/test/cpu_reader_support.cc
index 7554346..0a778ca 100644
--- a/src/ftrace_reader/test/cpu_reader_support.cc
+++ b/src/ftrace_reader/test/cpu_reader_support.cc
@@ -16,6 +16,7 @@
#include "src/ftrace_reader/test/cpu_reader_support.h"
+#include "perfetto/base/utils.h"
#include "src/ftrace_reader/ftrace_procfs.h"
#include <string.h>
@@ -23,8 +24,6 @@
namespace perfetto {
namespace {
-constexpr size_t kPageSize = 4096;
-
std::map<std::string, std::unique_ptr<ProtoTranslationTable>>* g_tables;
} // namespace
@@ -44,9 +43,9 @@
}
std::unique_ptr<uint8_t[]> PageFromXxd(const std::string& text) {
- auto buffer = std::unique_ptr<uint8_t[]>(new uint8_t[kPageSize]);
+ auto buffer = std::unique_ptr<uint8_t[]>(new uint8_t[base::kPageSize]);
const char* ptr = text.data();
- memset(buffer.get(), 0xfa, kPageSize);
+ memset(buffer.get(), 0xfa, base::kPageSize);
uint8_t* out = buffer.get();
while (*ptr != '\0') {
if (*(ptr++) != ':')
diff --git a/src/protozero/BUILD.gn b/src/protozero/BUILD.gn
index 0d67278..c7f3d95 100644
--- a/src/protozero/BUILD.gn
+++ b/src/protozero/BUILD.gn
@@ -30,6 +30,7 @@
"message.cc",
"message_handle.cc",
"proto_utils.cc",
+ "scattered_stream_null_delegate.cc",
"scattered_stream_writer.cc",
]
}
diff --git a/src/ftrace_reader/test/scattered_stream_null_delegate.cc b/src/protozero/scattered_stream_null_delegate.cc
similarity index 60%
rename from src/ftrace_reader/test/scattered_stream_null_delegate.cc
rename to src/protozero/scattered_stream_null_delegate.cc
index 562a3db..3587dba 100644
--- a/src/ftrace_reader/test/scattered_stream_null_delegate.cc
+++ b/src/protozero/scattered_stream_null_delegate.cc
@@ -14,22 +14,24 @@
* limitations under the License.
*/
-#include "src/ftrace_reader/test/scattered_stream_null_delegate.h"
+#include "perfetto/protozero/scattered_stream_null_delegate.h"
-namespace perfetto {
+namespace protozero {
// An implementation of ScatteredStreamWriter::Delegate which always returns
-// the same bit of memory (to better measure performance of users of
-// ScatteredStreamWriter without noisy allocations).
-
-ScatteredStreamNullDelegate::ScatteredStreamNullDelegate(size_t chunk_size)
+// the same piece of memory.
+// This is used when we need to no-op the writers (e.g. during teardown or in
+// case of resource exhaustion), avoiding that the clients have to deal with
+// nullptr checks.
+ScatteredStreamWriterNullDelegate::ScatteredStreamWriterNullDelegate(
+ size_t chunk_size)
: chunk_size_(chunk_size),
chunk_(std::unique_ptr<uint8_t[]>(new uint8_t[chunk_size_])){};
-ScatteredStreamNullDelegate::~ScatteredStreamNullDelegate() {}
+ScatteredStreamWriterNullDelegate::~ScatteredStreamWriterNullDelegate() {}
-protozero::ContiguousMemoryRange ScatteredStreamNullDelegate::GetNewBuffer() {
+ContiguousMemoryRange ScatteredStreamWriterNullDelegate::GetNewBuffer() {
return {chunk_.get(), chunk_.get() + chunk_size_};
}
-} // namespace perfetto
+} // namespace protozero
diff --git a/src/tracing/BUILD.gn b/src/tracing/BUILD.gn
index eef490d..80aedb4 100644
--- a/src/tracing/BUILD.gn
+++ b/src/tracing/BUILD.gn
@@ -37,6 +37,8 @@
"core/ftrace_config.cc",
"core/id_allocator.cc",
"core/id_allocator.h",
+ "core/null_trace_writer.cc",
+ "core/null_trace_writer.h",
"core/packet_stream_validator.cc",
"core/packet_stream_validator.h",
"core/patch_list.h",
@@ -122,6 +124,7 @@
]
sources = [
"core/id_allocator_unittest.cc",
+ "core/null_trace_writer_unittest.cc",
"core/packet_stream_validator_unittest.cc",
"core/patch_list_unittest.cc",
"core/service_impl_unittest.cc",
diff --git a/src/tracing/core/null_trace_writer.cc b/src/tracing/core/null_trace_writer.cc
new file mode 100644
index 0000000..36cddf4
--- /dev/null
+++ b/src/tracing/core/null_trace_writer.cc
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2018 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/core/null_trace_writer.h"
+
+#include "perfetto/base/logging.h"
+#include "perfetto/base/utils.h"
+
+#include "perfetto/protozero/message.h"
+
+#include "perfetto/trace/trace_packet.pbzero.h"
+
+namespace perfetto {
+
+NullTraceWriter::NullTraceWriter()
+ : delegate_(base::kPageSize), stream_(&delegate_) {
+ cur_packet_.reset(new protos::pbzero::TracePacket());
+ cur_packet_->Finalize(); // To avoid the DCHECK in NewTracePacket().
+}
+
+NullTraceWriter::~NullTraceWriter() {}
+
+void NullTraceWriter::Flush(std::function<void()> callback) {
+ // Flush() cannot be called in the middle of a TracePacket.
+ PERFETTO_CHECK(cur_packet_->is_finalized());
+
+ if (callback)
+ callback();
+}
+
+NullTraceWriter::TracePacketHandle NullTraceWriter::NewTracePacket() {
+ // If we hit this, the caller is calling NewTracePacket() without having
+ // finalized the previous packet.
+ PERFETTO_DCHECK(cur_packet_->is_finalized());
+ cur_packet_->Reset(&stream_);
+ return TraceWriter::TracePacketHandle(cur_packet_.get());
+}
+
+WriterID NullTraceWriter::writer_id() const {
+ return 0;
+}
+
+} // namespace perfetto
diff --git a/src/tracing/core/null_trace_writer.h b/src/tracing/core/null_trace_writer.h
new file mode 100644
index 0000000..8f10819
--- /dev/null
+++ b/src/tracing/core/null_trace_writer.h
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2018 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_CORE_NULL_TRACE_WRITER_H_
+#define SRC_TRACING_CORE_NULL_TRACE_WRITER_H_
+
+#include "perfetto/protozero/message_handle.h"
+#include "perfetto/protozero/scattered_stream_null_delegate.h"
+#include "perfetto/tracing/core/trace_writer.h"
+
+namespace perfetto {
+
+// A specialization of TraceWriter which no-ops all the writes routing them
+// into a fixed region of memory
+// See //include/perfetto/tracing/core/trace_writer.h for docs.
+class NullTraceWriter : public TraceWriter {
+ public:
+ NullTraceWriter();
+ ~NullTraceWriter() override;
+
+ // TraceWriter implementation. See documentation in trace_writer.h.
+ // TracePacketHandle is defined in trace_writer.h
+ TracePacketHandle NewTracePacket() override;
+ void Flush(std::function<void()> callback = {}) override;
+ WriterID writer_id() const override;
+
+ private:
+ NullTraceWriter(const NullTraceWriter&) = delete;
+ NullTraceWriter& operator=(const NullTraceWriter&) = delete;
+
+ protozero::ScatteredStreamWriterNullDelegate delegate_;
+ protozero::ScatteredStreamWriter stream_;
+
+ // The packet returned via NewTracePacket(). Its owned by this class,
+ // TracePacketHandle has just a pointer to it.
+ std::unique_ptr<protos::pbzero::TracePacket> cur_packet_;
+};
+
+} // namespace perfetto
+
+#endif // SRC_TRACING_CORE_NULL_TRACE_WRITER_H_
diff --git a/src/tracing/core/null_trace_writer_unittest.cc b/src/tracing/core/null_trace_writer_unittest.cc
new file mode 100644
index 0000000..f2d2f54
--- /dev/null
+++ b/src/tracing/core/null_trace_writer_unittest.cc
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2018 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/core/null_trace_writer.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include "perfetto/base/utils.h"
+#include "perfetto/tracing/core/trace_writer.h"
+
+#include "perfetto/trace/test_event.pbzero.h"
+#include "perfetto/trace/trace_packet.pbzero.h"
+
+namespace perfetto {
+namespace {
+
+TEST(NullTraceWriterTest, WriterIdIsZero) {
+ NullTraceWriter writer;
+ EXPECT_EQ(writer.writer_id(), 0);
+}
+
+TEST(NullTraceWriterTest, Writing) {
+ NullTraceWriter writer;
+ for (size_t i = 0; i < 3 * base::kPageSize; i++) {
+ auto packet = writer.NewTracePacket();
+ packet->set_for_testing()->set_str("Hello, world!");
+ }
+}
+
+TEST(NullTraceWriterTest, FlushCallbackIsCalled) {
+ NullTraceWriter writer;
+ writer.Flush();
+ bool was_called = false;
+ writer.Flush([&was_called] { was_called = true; });
+ EXPECT_TRUE(was_called);
+}
+
+} // namespace
+} // namespace perfetto
diff --git a/src/tracing/core/shared_memory_arbiter_impl.cc b/src/tracing/core/shared_memory_arbiter_impl.cc
index c1b7e23..c500936 100644
--- a/src/tracing/core/shared_memory_arbiter_impl.cc
+++ b/src/tracing/core/shared_memory_arbiter_impl.cc
@@ -20,6 +20,7 @@
#include "perfetto/base/task_runner.h"
#include "perfetto/tracing/core/commit_data_request.h"
#include "perfetto/tracing/core/shared_memory.h"
+#include "src/tracing/core/null_trace_writer.h"
#include "src/tracing/core/trace_writer_impl.h"
#include <limits>
@@ -240,8 +241,10 @@
std::lock_guard<std::mutex> scoped_lock(lock_);
id = active_writer_ids_.Allocate();
}
+ if (!id)
+ return std::unique_ptr<TraceWriter>(new NullTraceWriter());
return std::unique_ptr<TraceWriter>(
- id ? new TraceWriterImpl(this, id, target_buffer) : nullptr);
+ new TraceWriterImpl(this, id, target_buffer));
}
void SharedMemoryArbiterImpl::ReleaseWriterID(WriterID id) {
diff --git a/src/tracing/core/shared_memory_arbiter_impl_unittest.cc b/src/tracing/core/shared_memory_arbiter_impl_unittest.cc
index 8505876..a45328c 100644
--- a/src/tracing/core/shared_memory_arbiter_impl_unittest.cc
+++ b/src/tracing/core/shared_memory_arbiter_impl_unittest.cc
@@ -137,8 +137,9 @@
ASSERT_TRUE(writers.emplace(writer_id, std::move(writer)).second);
}
- // A further call should fail as we exhausted writer IDs.
- ASSERT_EQ(nullptr, arbiter_->CreateTraceWriter(0).get());
+ // A further call should return a null impl of trace writer as we exhausted
+ // writer IDs.
+ ASSERT_EQ(arbiter_->CreateTraceWriter(0)->writer_id(), 0);
}
// TODO(primiano): add multi-threaded tests.
diff --git a/src/tracing/core/trace_writer_impl.cc b/src/tracing/core/trace_writer_impl.cc
index ded5909..bfdf1f4 100644
--- a/src/tracing/core/trace_writer_impl.cc
+++ b/src/tracing/core/trace_writer_impl.cc
@@ -197,7 +197,7 @@
WriterID TraceWriterImpl::writer_id() const {
return id_;
-};
+}
// Base class ctor/dtor definition.
TraceWriter::TraceWriter() = default;
diff --git a/src/tracing/core/trace_writer_impl.h b/src/tracing/core/trace_writer_impl.h
index 28fa95f..3ba878d 100644
--- a/src/tracing/core/trace_writer_impl.h
+++ b/src/tracing/core/trace_writer_impl.h
@@ -36,7 +36,7 @@
TraceWriterImpl(SharedMemoryArbiterImpl*, WriterID, BufferID);
~TraceWriterImpl() override;
- // TraceWriter implementation. See documentation in trace_writer.h .
+ // TraceWriter implementation. See documentation in trace_writer.h.
TracePacketHandle NewTracePacket() override;
void Flush(std::function<void()> callback = {}) override;
WriterID writer_id() const override;