From 30ac4ce7115651fc70fec5e9c77523bf86c8489a Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Sat, 31 May 2025 15:03:35 +0200 Subject: [PATCH] NanotraceHR: Fix access after delete Ensure that the file is only deleted after every queye is finished. Change-Id: Iaa14bf670322afc05f3a91ed98a05ba12c3d1087 Reviewed-by: Thomas Hartmann --- src/libs/nanotrace/nanotracehr.cpp | 48 +++++++++---------- src/libs/nanotrace/nanotracehr.h | 21 ++------ src/libs/sqlite/sqlitetracing.cpp | 4 +- src/libs/sqlite/sqlitetracing.h | 4 +- .../tracing/qmldesignertracing.cpp | 4 +- 5 files changed, 34 insertions(+), 47 deletions(-) diff --git a/src/libs/nanotrace/nanotracehr.cpp b/src/libs/nanotrace/nanotracehr.cpp index 61d49efe3fd..e6bb03196da 100644 --- a/src/libs/nanotrace/nanotracehr.cpp +++ b/src/libs/nanotrace/nanotracehr.cpp @@ -51,11 +51,6 @@ constexpr bool isArgumentValid(const StaticString &string) return string.isValid() && string.size(); } -constexpr bool isArgumentValid(const std::identity &) -{ - return false; -} - template constexpr bool isArgumentValid(const String &string) { @@ -159,13 +154,13 @@ template NANOTRACE_EXPORT void convertToString(ArgumentsString &string, const QI template void flushEvents(const Utils::span events, std::thread::id threadId, - EnabledEventQueue &eventQueue) + std::shared_ptr file) { if (events.empty()) return; - std::lock_guard lock{eventQueue.file.fileMutex}; - auto &out = eventQueue.file.out; + std::lock_guard lock{file->fileMutex}; + auto &out = file->out; if (out.is_open()) { auto processId = QCoreApplication::applicationPid(); @@ -176,14 +171,13 @@ void flushEvents(const Utils::span events, } } -template NANOTRACE_EXPORT void flushEvents( - const Utils::span events, - std::thread::id threadId, - EnabledEventQueue &eventQueue); +template NANOTRACE_EXPORT void flushEvents(const Utils::span events, + std::thread::id threadId, + std::shared_ptr file); template NANOTRACE_EXPORT void flushEvents(const Utils::span events, std::thread::id threadId, - EnabledEventQueue &eventQueue); + std::shared_ptr file); void openFile(EnabledTraceFile &file) { @@ -214,17 +208,18 @@ void finalizeFile(EnabledTraceFile &file) template void flushInThread(EnabledEventQueue &eventQueue) { - if (eventQueue.file.processing.valid()) - eventQueue.file.processing.wait(); + if (eventQueue.file->processing.valid()) + eventQueue.file->processing.wait(); - auto flush = [&](const Utils::span &events, std::thread::id threadId) { - flushEvents(events, threadId, eventQueue); - }; + auto flush = [](const Utils::span &events, + std::thread::id threadId, + std::shared_ptr file) { flushEvents(events, threadId, file); }; - eventQueue.file.processing = std::async(std::launch::async, - flush, - eventQueue.currentEvents.subspan(0, eventQueue.eventsIndex), - eventQueue.threadId); + eventQueue.file->processing = std::async(std::launch::async, + flush, + eventQueue.currentEvents.subspan(0, eventQueue.eventsIndex), + eventQueue.threadId, + eventQueue.file); eventQueue.currentEvents = eventQueue.currentEvents.data() == eventQueue.eventsOne.data() ? eventQueue.eventsTwo : eventQueue.eventsOne; @@ -236,8 +231,8 @@ template NANOTRACE_EXPORT void flushInThread( template NANOTRACE_EXPORT void flushInThread(EnabledEventQueue &eventQueue); template -EventQueue::EventQueue(EnabledTraceFile &file) - : file{file} +EventQueue::EventQueue(std::shared_ptr file) + : file{std::move(file)} , threadId{std::this_thread::get_id()} { setEventsSpans(*eventArrayOne.get(), *eventArrayTwo.get()); @@ -245,7 +240,7 @@ EventQueue::EventQueue(EnabledTraceFile &file) if (QThread::currentThread()) { auto name = getThreadName(); if (name.size()) { - writeMetaEvent(file, "thread_name", name); + writeMetaEvent(*this->file, "thread_name", name); } } } @@ -272,9 +267,10 @@ void EventQueue::flush() { std::lock_guard lock{mutex}; if (isEnabled == IsEnabled::Yes && eventsIndex > 0) { - flushEvents(currentEvents.subspan(0, eventsIndex), threadId, *this); + flushEvents(currentEvents.subspan(0, eventsIndex), threadId, file); eventsIndex = 0; } + file.reset(); } template class NANOTRACE_EXPORT_TEMPLATE diff --git a/src/libs/nanotrace/nanotracehr.h b/src/libs/nanotrace/nanotracehr.h index d48b5cf34c6..92041f2af8b 100644 --- a/src/libs/nanotrace/nanotracehr.h +++ b/src/libs/nanotrace/nanotracehr.h @@ -433,19 +433,6 @@ using EnabledEventQueue = EventQueue; using EnabledEventQueueWithoutArguments = EventQueue; -template -void flushEvents(const Utils::span events, - std::thread::id threadId, - EnabledEventQueue &eventQueue); -extern template NANOTRACE_EXPORT void flushEvents( - const Utils::span events, - std::thread::id threadId, - EnabledEventQueue &eventQueue); -extern template NANOTRACE_EXPORT void flushEvents( - const Utils::span events, - std::thread::id threadId, - EnabledEventQueue &eventQueue); - template void flushInThread(EnabledEventQueue &eventQueue); extern template NANOTRACE_EXPORT void flushInThread( @@ -501,7 +488,9 @@ class EventQueue public: using IsActive = std::false_type; - template EventQueue(TraceFile &) {} + template + EventQueue(std::shared_ptr) + {} }; namespace Internal { @@ -583,7 +572,7 @@ class EventQueue public: using IsActive = std::true_type; - EventQueue(EnabledTraceFile &file); + EventQueue(std::shared_ptr file); ~EventQueue(); @@ -596,7 +585,7 @@ public: EventQueue &operator=(const EventQueue &) = delete; EventQueue &operator=(EventQueue &&) = delete; - EnabledTraceFile &file; + std::shared_ptr file; std::unique_ptr eventArrayOne = std::make_unique(); std::unique_ptr eventArrayTwo = std::make_unique(); TraceEventsSpan eventsOne; diff --git a/src/libs/sqlite/sqlitetracing.cpp b/src/libs/sqlite/sqlitetracing.cpp index 7070f3f500d..2a8c994e971 100644 --- a/src/libs/sqlite/sqlitetracing.cpp +++ b/src/libs/sqlite/sqlitetracing.cpp @@ -5,9 +5,9 @@ namespace Sqlite { -TraceFile &traceFile() +std::shared_ptr traceFile() { - static TraceFile traceFile{"tracing.json"}; + static auto traceFile = std::make_shared("tracing.json"); return traceFile; } diff --git a/src/libs/sqlite/sqlitetracing.h b/src/libs/sqlite/sqlitetracing.h index 8dadc6de0db..e55d76764e6 100644 --- a/src/libs/sqlite/sqlitetracing.h +++ b/src/libs/sqlite/sqlitetracing.h @@ -5,6 +5,8 @@ #include +#include + namespace Sqlite { using namespace NanotraceHR::Literals; @@ -19,7 +21,7 @@ constexpr NanotraceHR::Tracing sqliteTracingStatus() using TraceFile = NanotraceHR::TraceFile; -SQLITE_EXPORT TraceFile &traceFile(); +SQLITE_EXPORT std::shared_ptr traceFile(); NanotraceHR::StringViewWithStringArgumentsCategory &sqliteLowLevelCategory(); diff --git a/src/plugins/qmldesigner/libs/designercore/tracing/qmldesignertracing.cpp b/src/plugins/qmldesigner/libs/designercore/tracing/qmldesignertracing.cpp index 46c0f65a444..c66456a6151 100644 --- a/src/plugins/qmldesigner/libs/designercore/tracing/qmldesignertracing.cpp +++ b/src/plugins/qmldesigner/libs/designercore/tracing/qmldesignertracing.cpp @@ -15,12 +15,12 @@ namespace { using TraceFile = NanotraceHR::TraceFile; -auto &traceFile() +auto traceFile() { if constexpr (std::is_same_v) { return Sqlite::traceFile(); } else { - static TraceFile traceFile{"tracing.json"}; + static auto traceFile = std::make_shared("tracing.json"); return traceFile; } }