Fix a logger issue when disk is full on startup
There's an null dereference in the code when the logger fails to
create files.
This is happening because of this line of code in
`third_party/aos/aos/events/logging/logfile_utils.h` line 109:
WriteStats *WriteStatistics() const { return log_sink_->WriteStatistics(); }
The `log_sink_` pointer doesn't point to anything when the
`DetachedBufferWriter` is constructed with the special
`already_out_of_space_t` marker.
This patch fixes that by instantiating a dummy `OutOfDiskSpaceLogSink`
implementation of `LogSink` when `DetachedBufferWriter` is constructed
with the special `already_out_of_space_t` marker. This dummy
implementation refuses to do anything useful and just reports back
that the disk is full.
I also took this opportunity to mark the `DetachedBufferWriter`
destructor as `virtual`. This is imperative because it's inherited
from in at least one place.
Change-Id: I780dcd5a9418f11216da7cc96e3685e35190e998
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>
diff --git a/aos/events/logging/logfile_utils.cc b/aos/events/logging/logfile_utils.cc
index 36bac42..ec6bff3 100644
--- a/aos/events/logging/logfile_utils.cc
+++ b/aos/events/logging/logfile_utils.cc
@@ -111,6 +111,26 @@
*os << "null";
}
}
+
+// A dummy LogSink implementation that handles the special case when we create
+// a DetachedBufferWriter when there's no space left on the system. The
+// DetachedBufferWriter frequently dereferences log_sink_, so we want a class
+// here that effectively refuses to do anything meaningful.
+class OutOfDiskSpaceLogSink : public LogSink {
+ public:
+ WriteCode OpenForWrite() override { return WriteCode::kOutOfSpace; }
+ WriteCode Close() override { return WriteCode::kOk; }
+ bool is_open() const override { return false; }
+ WriteResult Write(
+ const absl::Span<const absl::Span<const uint8_t>> &) override {
+ return WriteResult{
+ .code = WriteCode::kOutOfSpace,
+ .messages_written = 0,
+ };
+ }
+ std::string_view name() const override { return "<out_of_disk_space>"; }
+};
+
} // namespace
DetachedBufferWriter::DetachedBufferWriter(std::unique_ptr<LogSink> log_sink,
@@ -123,6 +143,10 @@
}
}
+DetachedBufferWriter::DetachedBufferWriter(already_out_of_space_t)
+ : DetachedBufferWriter(std::make_unique<OutOfDiskSpaceLogSink>(), nullptr) {
+}
+
DetachedBufferWriter::~DetachedBufferWriter() {
Close();
if (ran_out_of_space_) {
diff --git a/aos/events/logging/logfile_utils.h b/aos/events/logging/logfile_utils.h
index 9dc7d88..18902d3 100644
--- a/aos/events/logging/logfile_utils.h
+++ b/aos/events/logging/logfile_utils.h
@@ -56,11 +56,11 @@
std::unique_ptr<DataEncoder> encoder);
// Creates a dummy instance which won't even open a file. It will act as if
// opening the file ran out of space immediately.
- DetachedBufferWriter(already_out_of_space_t) : ran_out_of_space_(true) {}
+ DetachedBufferWriter(already_out_of_space_t);
DetachedBufferWriter(DetachedBufferWriter &&other);
DetachedBufferWriter(const DetachedBufferWriter &) = delete;
- ~DetachedBufferWriter();
+ virtual ~DetachedBufferWriter();
DetachedBufferWriter &operator=(DetachedBufferWriter &&other);
DetachedBufferWriter &operator=(const DetachedBufferWriter &) = delete;