NanotraceHR: Fix access after delete

Ensure that the file is only deleted after every queye is finished.

Change-Id: Iaa14bf670322afc05f3a91ed98a05ba12c3d1087
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>
This commit is contained in:
Marco Bubke
2025-05-31 15:03:35 +02:00
parent 325e2b4056
commit 30ac4ce711
5 changed files with 34 additions and 47 deletions

View File

@@ -51,11 +51,6 @@ constexpr bool isArgumentValid(const StaticString<capacity> &string)
return string.isValid() && string.size(); return string.isValid() && string.size();
} }
constexpr bool isArgumentValid(const std::identity &)
{
return false;
}
template<typename String> template<typename String>
constexpr bool isArgumentValid(const String &string) constexpr bool isArgumentValid(const String &string)
{ {
@@ -159,13 +154,13 @@ template NANOTRACE_EXPORT void convertToString(ArgumentsString &string, const QI
template<typename TraceEvent> template<typename TraceEvent>
void flushEvents(const Utils::span<TraceEvent> events, void flushEvents(const Utils::span<TraceEvent> events,
std::thread::id threadId, std::thread::id threadId,
EnabledEventQueue<TraceEvent> &eventQueue) std::shared_ptr<EnabledTraceFile> file)
{ {
if (events.empty()) if (events.empty())
return; return;
std::lock_guard lock{eventQueue.file.fileMutex}; std::lock_guard lock{file->fileMutex};
auto &out = eventQueue.file.out; auto &out = file->out;
if (out.is_open()) { if (out.is_open()) {
auto processId = QCoreApplication::applicationPid(); auto processId = QCoreApplication::applicationPid();
@@ -176,14 +171,13 @@ void flushEvents(const Utils::span<TraceEvent> events,
} }
} }
template NANOTRACE_EXPORT void flushEvents( template NANOTRACE_EXPORT void flushEvents(const Utils::span<StringViewWithStringArgumentsTraceEvent> events,
const Utils::span<StringViewWithStringArgumentsTraceEvent> events,
std::thread::id threadId, std::thread::id threadId,
EnabledEventQueue<StringViewWithStringArgumentsTraceEvent> &eventQueue); std::shared_ptr<EnabledTraceFile> file);
template NANOTRACE_EXPORT void flushEvents(const Utils::span<TraceEventWithoutArguments> events, template NANOTRACE_EXPORT void flushEvents(const Utils::span<TraceEventWithoutArguments> events,
std::thread::id threadId, std::thread::id threadId,
EnabledEventQueue<TraceEventWithoutArguments> &eventQueue); std::shared_ptr<EnabledTraceFile> file);
void openFile(EnabledTraceFile &file) void openFile(EnabledTraceFile &file)
{ {
@@ -214,17 +208,18 @@ void finalizeFile(EnabledTraceFile &file)
template<typename TraceEvent> template<typename TraceEvent>
void flushInThread(EnabledEventQueue<TraceEvent> &eventQueue) void flushInThread(EnabledEventQueue<TraceEvent> &eventQueue)
{ {
if (eventQueue.file.processing.valid()) if (eventQueue.file->processing.valid())
eventQueue.file.processing.wait(); eventQueue.file->processing.wait();
auto flush = [&](const Utils::span<TraceEvent> &events, std::thread::id threadId) { auto flush = [](const Utils::span<TraceEvent> &events,
flushEvents(events, threadId, eventQueue); std::thread::id threadId,
}; std::shared_ptr<EnabledTraceFile> file) { flushEvents(events, threadId, file); };
eventQueue.file.processing = std::async(std::launch::async, eventQueue.file->processing = std::async(std::launch::async,
flush, flush,
eventQueue.currentEvents.subspan(0, eventQueue.eventsIndex), eventQueue.currentEvents.subspan(0, eventQueue.eventsIndex),
eventQueue.threadId); eventQueue.threadId,
eventQueue.file);
eventQueue.currentEvents = eventQueue.currentEvents.data() == eventQueue.eventsOne.data() eventQueue.currentEvents = eventQueue.currentEvents.data() == eventQueue.eventsOne.data()
? eventQueue.eventsTwo ? eventQueue.eventsTwo
: eventQueue.eventsOne; : eventQueue.eventsOne;
@@ -236,8 +231,8 @@ template NANOTRACE_EXPORT void flushInThread(
template NANOTRACE_EXPORT void flushInThread(EnabledEventQueue<TraceEventWithoutArguments> &eventQueue); template NANOTRACE_EXPORT void flushInThread(EnabledEventQueue<TraceEventWithoutArguments> &eventQueue);
template<typename TraceEvent> template<typename TraceEvent>
EventQueue<TraceEvent, Tracing::IsEnabled>::EventQueue(EnabledTraceFile &file) EventQueue<TraceEvent, Tracing::IsEnabled>::EventQueue(std::shared_ptr<EnabledTraceFile> file)
: file{file} : file{std::move(file)}
, threadId{std::this_thread::get_id()} , threadId{std::this_thread::get_id()}
{ {
setEventsSpans(*eventArrayOne.get(), *eventArrayTwo.get()); setEventsSpans(*eventArrayOne.get(), *eventArrayTwo.get());
@@ -245,7 +240,7 @@ EventQueue<TraceEvent, Tracing::IsEnabled>::EventQueue(EnabledTraceFile &file)
if (QThread::currentThread()) { if (QThread::currentThread()) {
auto name = getThreadName(); auto name = getThreadName();
if (name.size()) { if (name.size()) {
writeMetaEvent(file, "thread_name", name); writeMetaEvent(*this->file, "thread_name", name);
} }
} }
} }
@@ -272,9 +267,10 @@ void EventQueue<TraceEvent, Tracing::IsEnabled>::flush()
{ {
std::lock_guard lock{mutex}; std::lock_guard lock{mutex};
if (isEnabled == IsEnabled::Yes && eventsIndex > 0) { if (isEnabled == IsEnabled::Yes && eventsIndex > 0) {
flushEvents(currentEvents.subspan(0, eventsIndex), threadId, *this); flushEvents(currentEvents.subspan(0, eventsIndex), threadId, file);
eventsIndex = 0; eventsIndex = 0;
} }
file.reset();
} }
template class NANOTRACE_EXPORT_TEMPLATE template class NANOTRACE_EXPORT_TEMPLATE

View File

@@ -433,19 +433,6 @@ using EnabledEventQueue = EventQueue<TraceEvent, Tracing::IsEnabled>;
using EnabledEventQueueWithoutArguments = EventQueue<TraceEventWithoutArguments, Tracing::IsEnabled>; using EnabledEventQueueWithoutArguments = EventQueue<TraceEventWithoutArguments, Tracing::IsEnabled>;
template<typename TraceEvent>
void flushEvents(const Utils::span<TraceEvent> events,
std::thread::id threadId,
EnabledEventQueue<TraceEvent> &eventQueue);
extern template NANOTRACE_EXPORT void flushEvents(
const Utils::span<StringViewWithStringArgumentsTraceEvent> events,
std::thread::id threadId,
EnabledEventQueue<StringViewWithStringArgumentsTraceEvent> &eventQueue);
extern template NANOTRACE_EXPORT void flushEvents(
const Utils::span<TraceEventWithoutArguments> events,
std::thread::id threadId,
EnabledEventQueue<TraceEventWithoutArguments> &eventQueue);
template<typename TraceEvent> template<typename TraceEvent>
void flushInThread(EnabledEventQueue<TraceEvent> &eventQueue); void flushInThread(EnabledEventQueue<TraceEvent> &eventQueue);
extern template NANOTRACE_EXPORT void flushInThread( extern template NANOTRACE_EXPORT void flushInThread(
@@ -501,7 +488,9 @@ class EventQueue
public: public:
using IsActive = std::false_type; using IsActive = std::false_type;
template<typename TraceFile> EventQueue(TraceFile &) {} template<typename TraceFile>
EventQueue(std::shared_ptr<TraceFile>)
{}
}; };
namespace Internal { namespace Internal {
@@ -583,7 +572,7 @@ class EventQueue<TraceEvent, Tracing::IsEnabled>
public: public:
using IsActive = std::true_type; using IsActive = std::true_type;
EventQueue(EnabledTraceFile &file); EventQueue(std::shared_ptr<EnabledTraceFile> file);
~EventQueue(); ~EventQueue();
@@ -596,7 +585,7 @@ public:
EventQueue &operator=(const EventQueue &) = delete; EventQueue &operator=(const EventQueue &) = delete;
EventQueue &operator=(EventQueue &&) = delete; EventQueue &operator=(EventQueue &&) = delete;
EnabledTraceFile &file; std::shared_ptr<EnabledTraceFile> file;
std::unique_ptr<TraceEvents> eventArrayOne = std::make_unique<TraceEvents>(); std::unique_ptr<TraceEvents> eventArrayOne = std::make_unique<TraceEvents>();
std::unique_ptr<TraceEvents> eventArrayTwo = std::make_unique<TraceEvents>(); std::unique_ptr<TraceEvents> eventArrayTwo = std::make_unique<TraceEvents>();
TraceEventsSpan eventsOne; TraceEventsSpan eventsOne;

View File

@@ -5,9 +5,9 @@
namespace Sqlite { namespace Sqlite {
TraceFile &traceFile() std::shared_ptr<TraceFile> traceFile()
{ {
static TraceFile traceFile{"tracing.json"}; static auto traceFile = std::make_shared<TraceFile>("tracing.json");
return traceFile; return traceFile;
} }

View File

@@ -5,6 +5,8 @@
#include <nanotrace/nanotracehr.h> #include <nanotrace/nanotracehr.h>
#include <memory>
namespace Sqlite { namespace Sqlite {
using namespace NanotraceHR::Literals; using namespace NanotraceHR::Literals;
@@ -19,7 +21,7 @@ constexpr NanotraceHR::Tracing sqliteTracingStatus()
using TraceFile = NanotraceHR::TraceFile<sqliteTracingStatus()>; using TraceFile = NanotraceHR::TraceFile<sqliteTracingStatus()>;
SQLITE_EXPORT TraceFile &traceFile(); SQLITE_EXPORT std::shared_ptr<TraceFile> traceFile();
NanotraceHR::StringViewWithStringArgumentsCategory<sqliteTracingStatus()> &sqliteLowLevelCategory(); NanotraceHR::StringViewWithStringArgumentsCategory<sqliteTracingStatus()> &sqliteLowLevelCategory();

View File

@@ -15,12 +15,12 @@ namespace {
using TraceFile = NanotraceHR::TraceFile<tracingStatus()>; using TraceFile = NanotraceHR::TraceFile<tracingStatus()>;
auto &traceFile() auto traceFile()
{ {
if constexpr (std::is_same_v<Sqlite::TraceFile, TraceFile>) { if constexpr (std::is_same_v<Sqlite::TraceFile, TraceFile>) {
return Sqlite::traceFile(); return Sqlite::traceFile();
} else { } else {
static TraceFile traceFile{"tracing.json"}; static auto traceFile = std::make_shared<TraceFile>("tracing.json");
return traceFile; return traceFile;
} }
} }