Skip to content

Commit

Permalink
Fix core dump when flush while the spsc queue doesn't have enough space.
Browse files Browse the repository at this point in the history
  • Loading branch information
Weissle authored and odygrd committed Mar 14, 2024
1 parent 21bd16a commit c718304
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 2 deletions.
10 changes: 8 additions & 2 deletions quill/include/quill/detail/LogManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,16 @@ class LogManager

ThreadContext* const thread_context =
_thread_context_collection.local_thread_context<QUILL_QUEUE_TYPE>();

auto& spsc_queue = thread_context->spsc_queue<QUILL_QUEUE_TYPE>();
size_t constexpr total_size = sizeof(Header) + sizeof(uintptr_t);

std::byte* write_buffer =
thread_context->spsc_queue<QUILL_QUEUE_TYPE>().prepare_write(static_cast<uint32_t>(total_size));
std::byte* write_buffer;
while ((write_buffer = spsc_queue.prepare_write(static_cast<uint32_t>(total_size))) == nullptr)
{
std::this_thread::sleep_for(std::chrono::nanoseconds{100});
}

std::byte const* const write_begin = write_buffer;

write_buffer = detail::align_pointer<alignof(Header), std::byte>(write_buffer);
Expand Down
1 change: 1 addition & 0 deletions quill/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ quill_add_test(TEST_TypeTraitsCopyable TypeTraitsCopyableTest.cpp)
quill_add_test(TEST_UnboundedQueueTest.cpp UnboundedQueueTest.cpp)
quill_add_test(TEST_Utilities UtilitiesTest.cpp)
quill_add_test(TEST_Utility UtilityTest.cpp)
quill_add_test(TEST_QuillBoundQueueLog QuillBoundQueueLogTest.cpp)

if (NOT QUILL_USE_VALGRIND)
quill_add_test(TEST_RdtscClock RdtscClockTest.cpp)
Expand Down
33 changes: 33 additions & 0 deletions quill/test/LogTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2473,4 +2473,37 @@ TEST_CASE("default_logger_with_very_large_random_strings")
quill::detail::remove_file(filename);
}

TEST_CASE("flush_without_any_log")
{
LogManager lm;

quill::Config cfg;

lm.configure(cfg);

lm.start_backend_worker(false, std::initializer_list<int32_t>{});

Logger* default_logger = lm.logger_collection().get_logger();

std::atomic_bool is_flush_done = false;

std::thread watcher(
[&is_flush_done]()
{
int try_count = 3;
while (is_flush_done.load() == false && try_count--)
{
std::this_thread::sleep_for(std::chrono::seconds(1));
}
REQUIRE_EQ(is_flush_done.load(), true);
});

lm.flush();

is_flush_done = true;

watcher.join();

lm.stop_backend_worker();
}
TEST_SUITE_END();
52 changes: 52 additions & 0 deletions quill/test/QuillBoundQueueLogTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#include <iostream>
#define QUILL_USE_BOUNDED_QUEUE

#include "doctest/doctest.h"

#include "quill/Quill.h"
#include "quill/detail/LogManager.h"

TEST_SUITE_BEGIN("QuillBoundQueueLog");

using namespace quill;
using namespace quill::detail;

TEST_CASE("quill_full_bound_queue_safe_flush")
{
fs::path const filename{"test_quill_full_bound_queue_safe_flush"};

LogManager lm;

quill::Config cfg;

cfg.default_handlers.emplace_back(lm.handler_collection().create_handler<FileHandler>(
filename.string(),
[]()
{
quill::FileHandlerConfig cfg;
cfg.set_open_mode('w');
return cfg;
}(),
FileEventNotifier{}));

lm.configure(cfg);

lm.start_backend_worker(false, std::initializer_list<int32_t>{});

Logger* default_logger = lm.logger_collection().get_logger();

for (int i = 0; i < 5000; ++i)
{
LOG_DEBUG(default_logger, "Log something to fulfill the bound queue.");
LOG_INFO(default_logger, "Log something to fulfill the bound queue.");
LOG_WARNING(default_logger, "Log something to fulfill the bound queue.");
LOG_ERROR(default_logger, "Log something to fulfill the bound queue.");
}

// Test is passed if not core dump.
lm.flush();

lm.stop_backend_worker();
}

TEST_SUITE_END();

0 comments on commit c718304

Please sign in to comment.