Skip to content

Commit 64c5d78

Browse files
committed
fix crash when log backtrace is used
1 parent f4b5afa commit 64c5d78

File tree

3 files changed

+60
-4
lines changed

3 files changed

+60
-4
lines changed

include/quill/backend/BackendWorker.h

+9-2
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,8 @@ class BackendWorker
528528
// Allocate a new TransitEvent or use an existing one to store the message from the queue
529529
TransitEvent* transit_event = thread_context->_transit_event_buffer->back();
530530

531+
assert(transit_event->formatted_msg);
532+
531533
std::memcpy(&transit_event->timestamp, read_pos, sizeof(transit_event->timestamp));
532534
read_pos += sizeof(transit_event->timestamp);
533535

@@ -828,9 +830,14 @@ class BackendWorker
828830
{
829831
if (transit_event.logger_base->backtrace_storage)
830832
{
831-
// this is a backtrace log and we will store it
833+
// this is a backtrace log and we will store the transit event
834+
// we need to pass a copy of transit_event here and not move the existing
835+
// the transit events are reused
836+
TransitEvent transit_event_copy;
837+
transit_event.copy_to(transit_event_copy);
838+
832839
transit_event.logger_base->backtrace_storage->store(
833-
std::move(transit_event), thread_context.thread_id(), thread_context.thread_name());
840+
std::move(transit_event_copy), thread_context.thread_id(), thread_context.thread_name());
834841
}
835842
else
836843
{

include/quill/backend/TransitEvent.h

+21-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@ struct TransitEvent
3939
/***/
4040
~TransitEvent() = default;
4141

42-
/***/
42+
/**
43+
* Copy constructor and assignment operator are deleted to prevent accidental copying.
44+
* Use the `clone` function for explicit and controlled duplication.
45+
*/
4346
TransitEvent(TransitEvent const& other) = delete;
4447
TransitEvent& operator=(TransitEvent const& other) = delete;
4548

@@ -72,6 +75,23 @@ struct TransitEvent
7275
return *this;
7376
}
7477

78+
/***/
79+
void copy_to(TransitEvent& other) const {
80+
other.timestamp = timestamp;
81+
other.macro_metadata = macro_metadata;
82+
other.logger_base = logger_base;
83+
other.flush_flag = flush_flag;
84+
other.dynamic_log_level = dynamic_log_level;
85+
86+
// manually copy the fmt::buffer
87+
other.formatted_msg->reserve(formatted_msg->size());
88+
other.formatted_msg->append(*formatted_msg);
89+
90+
if (this->named_args) {
91+
other.named_args = std::make_unique<std::vector<std::pair<std::string, std::string>>>(*this->named_args);
92+
}
93+
}
94+
7595
/***/
7696
QUILL_NODISCARD QUILL_ATTRIBUTE_HOT LogLevel log_level() const noexcept
7797
{

test/integration_tests/BacktraceFlushOnErrorTest.cpp

+30-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "quill/LogMacros.h"
77
#include "quill/sinks/FileSink.h"
88

9+
#include <chrono>
910
#include <cstdio>
1011
#include <string>
1112
#include <vector>
@@ -52,6 +53,19 @@ TEST_CASE("backtrace_flush_on_error")
5253
}
5354
LOG_ERROR(logger, "After error");
5455

56+
// flush here and resume test
57+
logger->flush_log();
58+
59+
// Enable backtrace for 2 messages for error
60+
logger->init_backtrace(4, LogLevel::Error);
61+
62+
for (size_t i = 0; i < 512; ++i)
63+
{
64+
LOG_BACKTRACE(logger, "More backtrace log {}", i);
65+
std::this_thread::sleep_for(std::chrono::milliseconds(1));
66+
}
67+
LOG_ERROR(logger, "After another error");
68+
5569
logger->flush_log();
5670
Frontend::remove_logger(logger);
5771

@@ -61,7 +75,7 @@ TEST_CASE("backtrace_flush_on_error")
6175
// Read file and check
6276
std::vector<std::string> const file_contents = quill::testing::file_contents(filename);
6377

64-
REQUIRE_EQ(file_contents.size(), 4);
78+
REQUIRE_EQ(file_contents.size(), 9);
6579

6680
std::string expected_string_1 = "LOG_INFO " + logger_name + " Before backtrace log";
6781
REQUIRE(quill::testing::file_contains(file_contents, expected_string_1));
@@ -75,5 +89,20 @@ TEST_CASE("backtrace_flush_on_error")
7589
std::string expected_string_4 = "LOG_BACKTRACE " + logger_name + " Backtrace log 11";
7690
REQUIRE(quill::testing::file_contains(file_contents, expected_string_4));
7791

92+
std::string expected_string_5 = "LOG_BACKTRACE " + logger_name + " More backtrace log 508";
93+
REQUIRE(quill::testing::file_contains(file_contents, expected_string_5));
94+
95+
std::string expected_string_6 = "LOG_BACKTRACE " + logger_name + " More backtrace log 509";
96+
REQUIRE(quill::testing::file_contains(file_contents, expected_string_6));
97+
98+
std::string expected_string_7 = "LOG_BACKTRACE " + logger_name + " More backtrace log 510";
99+
REQUIRE(quill::testing::file_contains(file_contents, expected_string_7));
100+
101+
std::string expected_string_8 = "LOG_BACKTRACE " + logger_name + " More backtrace log 511";
102+
REQUIRE(quill::testing::file_contains(file_contents, expected_string_8));
103+
104+
std::string expected_string_9 = "LOG_ERROR " + logger_name + " After another error";
105+
REQUIRE(quill::testing::file_contains(file_contents, expected_string_9));
106+
78107
testing::remove_file(filename);
79108
}

0 commit comments

Comments
 (0)