From 8fd4daa686e3edb9bff862411b818193d1911459 Mon Sep 17 00:00:00 2001 From: "Taylor, Raymond" Date: Thu, 16 Jul 2026 16:13:42 -0700 Subject: [PATCH 1/4] Fix test regression; enhance stability of test --- .../CommonUtilities/log/CopyDriver.cpp | 12 +- .../CommonUtilities/log/CopyDriver.h | 4 +- .../Core/source/infra/LogSetup.cpp | 3 +- IntelPresentMon/PresentMonAPI2/Internal.h | 17 ++- .../PresentMonAPI2/PresentMonAPI.cpp | 106 +++++++++++------- .../PresentMonAPI2Loader/Implementation.cpp | 21 ++++ .../PresentMonAPI2Tests/EtlTests.cpp | 2 - .../InterimBroadcasterTests.cpp | 10 +- .../PresentMonAPI2Tests/Logging.cpp | 23 ++-- .../PresentMonMiddleware/LogSetup.cpp | 25 ++++- .../PresentMonMiddleware/LogSetup.h | 4 +- IntelPresentMon/SampleClient/LogSetup.cpp | 3 +- 12 files changed, 149 insertions(+), 81 deletions(-) diff --git a/IntelPresentMon/CommonUtilities/log/CopyDriver.cpp b/IntelPresentMon/CommonUtilities/log/CopyDriver.cpp index 44eb3acac..01100f008 100644 --- a/IntelPresentMon/CommonUtilities/log/CopyDriver.cpp +++ b/IntelPresentMon/CommonUtilities/log/CopyDriver.cpp @@ -2,16 +2,20 @@ namespace pmon::util::log { - CopyDriver::CopyDriver(std::shared_ptr pChannel) + CopyDriver::CopyDriver(IChannel* pChannel) noexcept : - pChannel_{ std::move(pChannel) } + pChannel_{ pChannel } {} void CopyDriver::Submit(const Entry& e) { - pChannel_->Submit(e); + if (pChannel_) { + pChannel_->Submit(e); + } } void CopyDriver::Flush() { - pChannel_->Flush(); + if (pChannel_) { + pChannel_->Flush(); + } } } \ No newline at end of file diff --git a/IntelPresentMon/CommonUtilities/log/CopyDriver.h b/IntelPresentMon/CommonUtilities/log/CopyDriver.h index 9eef953ca..68e4589ce 100644 --- a/IntelPresentMon/CommonUtilities/log/CopyDriver.h +++ b/IntelPresentMon/CommonUtilities/log/CopyDriver.h @@ -8,10 +8,10 @@ namespace pmon::util::log class CopyDriver : public IDriver { public: - CopyDriver(std::shared_ptr pChannel); + explicit CopyDriver(IChannel* pChannel) noexcept; void Submit(const Entry&) override; void Flush() override; private: - std::shared_ptr pChannel_; + IChannel* pChannel_; }; } diff --git a/IntelPresentMon/Core/source/infra/LogSetup.cpp b/IntelPresentMon/Core/source/infra/LogSetup.cpp index 432438e58..267d1c5e3 100644 --- a/IntelPresentMon/Core/source/infra/LogSetup.cpp +++ b/IntelPresentMon/Core/source/infra/LogSetup.cpp @@ -131,8 +131,7 @@ namespace p2c // connect dll channel and id table to exe, get access to global settings in dll LoggingSingletons getters; if (linkMiddlewareLogs) { - getters = pmLinkLogging_(pChan, []() -> IdentificationTable& { - return IdentificationTable::Get_(); }); + getters = pmLinkLoggingPtrs_(pChan.get(), IdentificationTable::GetPtr()); } // set the global policy settings if (opt.logLevel) { diff --git a/IntelPresentMon/PresentMonAPI2/Internal.h b/IntelPresentMon/PresentMonAPI2/Internal.h index 1347bd211..d28925369 100644 --- a/IntelPresentMon/PresentMonAPI2/Internal.h +++ b/IntelPresentMon/PresentMonAPI2/Internal.h @@ -15,17 +15,22 @@ PRESENTMON_API2_EXPORT _CrtMemState pmCreateHeapCheckpoint_(); // log configuration support functions struct LoggingSingletons { - std::function getGlobalPolicy; - std::function getLineTable; + using GlobalPolicyRefFn = pmon::util::log::GlobalPolicy & (*)(); + using LineTableRefFn = pmon::util::log::LineTable & (*)(); + GlobalPolicyRefFn getGlobalPolicy = nullptr; + LineTableRefFn getLineTable = nullptr; operator bool() const noexcept { return getGlobalPolicy || getLineTable; } }; -// function to connect (subordinate) the dll logging system to the exe one -// replace default channel (nullptr) with a channel that copies entries to pChannel -// optionally hook up an id table to copy entries to as well -// return getters for config singletons in the dll to config from the exe +// Connect (subordinate) the dll logging system to the host module. +// Prefer pmLinkLoggingPtrs_ for all in-repo callers: do not pass shared_ptr or +// std::function across module boundaries (/MT heap). pmLinkLogging_ remains exported +// for older loader/middleware ABI (v2.5.1 mangled name); avoid new call sites. +PRESENTMON_API2_EXPORT LoggingSingletons pmLinkLoggingPtrs_( + pmon::util::log::IChannel* pChannel, + pmon::util::log::IdentificationTable* pExeTable); PRESENTMON_API2_EXPORT LoggingSingletons pmLinkLogging_( std::shared_ptr pChannel, std::function getIdTable); diff --git a/IntelPresentMon/PresentMonAPI2/PresentMonAPI.cpp b/IntelPresentMon/PresentMonAPI2/PresentMonAPI.cpp index 4bcdea195..aab4f6650 100644 --- a/IntelPresentMon/PresentMonAPI2/PresentMonAPI.cpp +++ b/IntelPresentMon/PresentMonAPI2/PresentMonAPI.cpp @@ -100,63 +100,87 @@ PRESENTMON_API2_EXPORT _CrtMemState pmCreateHeapCheckpoint_() return s; } -PRESENTMON_API2_EXPORT LoggingSingletons pmLinkLogging_( - std::shared_ptr pChannel, - std::function getIdTable) +namespace { - using namespace util::log; - // set api dll default logging channel to copy to exe logging channel - SetupCopyChannel(std::move(pChannel)); - // connecting id tables (dll => exe) - if (pLinkedIdTableSink_) { - IdentificationTable::UnregisterSink(pLinkedIdTableSink_.get()); - pLinkedIdTableSink_.reset(); + pmon::util::log::GlobalPolicy& GetMiddlewareGlobalPolicy_() + { + return pmon::util::log::GlobalPolicy::Get(); + } + + pmon::util::log::LineTable& GetMiddlewareLineTable_() + { + return pmon::util::log::LineTable::Get_(); } - if (getIdTable) { - class Sink : public IIdentificationSink - { - public: - Sink(std::function getTable) - : - getTable_{ std::move(getTable) } - {} - void AddThread(uint32_t tid, uint32_t pid, std::string name) override + + LoggingSingletons LinkLogging_(pmon::util::log::IChannel* pChannel, + pmon::util::log::IdentificationTable* pExeTable) + { + using namespace pmon::util::log; + SetupCopyChannel(pChannel); + if (pLinkedIdTableSink_) { + IdentificationTable::UnregisterSink(pLinkedIdTableSink_.get()); + pLinkedIdTableSink_.reset(); + } + if (pExeTable) { + class Sink : public IIdentificationSink { - getTable_().AddThread_(tid, pid, name); + public: + explicit Sink(IdentificationTable* pTable) noexcept + : + pTable_{ pTable } + {} + void AddThread(uint32_t tid, uint32_t pid, std::string name) override + { + pTable_->AddThread_(tid, pid, name); + } + void AddProcess(uint32_t pid, std::string name) override + { + pTable_->AddProcess_(pid, name); + } + private: + IdentificationTable* pTable_; + }; + pLinkedIdTableSink_ = std::make_shared(pExeTable); + IdentificationTable::RegisterSink(pLinkedIdTableSink_); + const auto bulk = IdentificationTable::GetBulk(); + for (auto& t : bulk.threads) { + pLinkedIdTableSink_->AddThread(t.tid, t.pid, t.name); } - void AddProcess(uint32_t pid, std::string name) override - { - getTable_().AddProcess_(pid, name); + for (auto& p : bulk.processes) { + pLinkedIdTableSink_->AddProcess(p.pid, p.name); } - private: - std::function getTable_; - }; - pLinkedIdTableSink_ = std::make_shared(getIdTable); - IdentificationTable::RegisterSink(pLinkedIdTableSink_); - const auto bulk = IdentificationTable::GetBulk(); - for (auto& t : bulk.threads) { - pLinkedIdTableSink_->AddThread(t.tid, t.pid, t.name); - } - for (auto& p : bulk.processes) { - pLinkedIdTableSink_->AddProcess(p.pid, p.name); } + return { + .getGlobalPolicy = &GetMiddlewareGlobalPolicy_, + .getLineTable = &GetMiddlewareLineTable_, + }; } - // return functions to access the global settings objects - return { - .getGlobalPolicy = []() -> GlobalPolicy& { return GlobalPolicy::Get(); }, - .getLineTable = []() -> LineTable& { return LineTable::Get_(); }, - }; +} + +PRESENTMON_API2_EXPORT LoggingSingletons pmLinkLoggingPtrs_( + pmon::util::log::IChannel* pChannel, + pmon::util::log::IdentificationTable* pExeTable) +{ + return LinkLogging_(pChannel, pExeTable); +} + +PRESENTMON_API2_EXPORT LoggingSingletons pmLinkLogging_( + std::shared_ptr pChannel, + std::function getIdTable) +{ + pmon::util::log::IdentificationTable* pExeTable = getIdTable ? &getIdTable() : nullptr; + return LinkLogging_(pChannel.get(), pExeTable); } PRESENTMON_API2_EXPORT void pmUnlinkLogging_() noexcept { using namespace util::log; - pmquell(FlushEntryPoint()) - pmquell(InjectDefaultChannel({})) if (pLinkedIdTableSink_) { pmquell(IdentificationTable::UnregisterSink(pLinkedIdTableSink_.get())) pLinkedIdTableSink_.reset(); } + pmquell(FlushEntryPoint()) + pmquell(SeverCopyLoggingBridge()) } PRESENTMON_API2_EXPORT void pmFlushEntryPoint_() noexcept diff --git a/IntelPresentMon/PresentMonAPI2Loader/Implementation.cpp b/IntelPresentMon/PresentMonAPI2Loader/Implementation.cpp index 3f2962bd6..b34d264f9 100644 --- a/IntelPresentMon/PresentMonAPI2Loader/Implementation.cpp +++ b/IntelPresentMon/PresentMonAPI2Loader/Implementation.cpp @@ -60,6 +60,8 @@ PM_STATUS(*pFunc_pmDiagnosticUnblockWaitingThread_)() = nullptr; _CrtMemState(*pFunc_pmCreateHeapCheckpoint__)() = nullptr; LoggingSingletons(*pFunc_pmLinkLogging__)(std::shared_ptr, std::function) = nullptr; +LoggingSingletons(*pFunc_pmLinkLoggingPtrs__)(pmon::util::log::IChannel*, + pmon::util::log::IdentificationTable*) = nullptr; void(*pFunc_pmUnlinkLogging__)() = nullptr; void(*pFunc_pmFlushEntryPoint__)() = nullptr; void(*pFunc_pmSetupODSLogging__)(PM_DIAGNOSTIC_LEVEL, PM_DIAGNOSTIC_LEVEL, bool) = nullptr; @@ -198,6 +200,7 @@ PRESENTMON_API2_EXPORT PM_STATUS LoadLibrary_(bool versionOnly = false) // internal (optional: older middleware may omit refactored hooks) RESOLVE_CPP_OPTIONAL(pmCreateHeapCheckpoint_); RESOLVE_CPP_OPTIONAL(pmLinkLogging_); + RESOLVE_CPP_OPTIONAL(pmLinkLoggingPtrs_); RESOLVE_CPP_OPTIONAL(pmUnlinkLogging_); RESOLVE_CPP_OPTIONAL(pmFlushEntryPoint_); RESOLVE_CPP_OPTIONAL(pmSetupODSLogging_); @@ -359,11 +362,29 @@ PRESENTMON_API2_EXPORT LoggingSingletons pmLinkLogging_(std::shared_ptr(controlPipe_); return true; } diff --git a/IntelPresentMon/PresentMonAPI2Tests/InterimBroadcasterTests.cpp b/IntelPresentMon/PresentMonAPI2Tests/InterimBroadcasterTests.cpp index 45efa6626..132a79b44 100644 --- a/IntelPresentMon/PresentMonAPI2Tests/InterimBroadcasterTests.cpp +++ b/IntelPresentMon/PresentMonAPI2Tests/InterimBroadcasterTests.cpp @@ -1397,8 +1397,8 @@ namespace InterimBroadcasterTests // setup query PM_BEGIN_FIXED_FRAME_QUERY(FQ) pmapi::FixedQueryElement timestamp{ this, PM_METRIC_CPU_START_QPC }; - pmapi::FixedQueryElement timeInPres{ this, PM_METRIC_IN_PRESENT_API }; - PM_END_FIXED_QUERY query{ session, 1'000 }; + pmapi::FixedQueryElement timeInPres{ this, PM_METRIC_IN_PRESENT_API }; + PM_END_FIXED_QUERY query{ session, 350 }; struct Row { uint64_t timestamp; double timeInPresent; }; std::vector frames; @@ -1418,15 +1418,17 @@ namespace InterimBroadcasterTests }; // verify that backpressure works correctly to ensure no frames are lost + std::this_thread::sleep_for(machine::ScaleWait(400ms)); + const auto count1 = consume(); Logger::WriteMessage(std::format("count [{}]\n", count1).c_str()); - std::this_thread::sleep_for(machine::ScaleWait(300ms)); + std::this_thread::sleep_for(machine::ScaleWait(400ms)); const auto count2 = consume(); Logger::WriteMessage(std::format("count [{}]\n", count2).c_str()); - std::this_thread::sleep_for(machine::ScaleWait(500ms)); + std::this_thread::sleep_for(machine::ScaleWait(150ms)); const auto count3 = consume(); Logger::WriteMessage(std::format("count [{}]\n", count3).c_str()); diff --git a/IntelPresentMon/PresentMonAPI2Tests/Logging.cpp b/IntelPresentMon/PresentMonAPI2Tests/Logging.cpp index 41d19998a..c31fe5375 100644 --- a/IntelPresentMon/PresentMonAPI2Tests/Logging.cpp +++ b/IntelPresentMon/PresentMonAPI2Tests/Logging.cpp @@ -64,7 +64,6 @@ namespace pmon::test { std::mutex mtx; bool linked = false; - LoggingSingletons getters{}; }; LogLinkState& GetLogLinkState_() @@ -73,6 +72,11 @@ namespace pmon::test return state; } + util::log::IdentificationTable* GetLinkIdTablePtr_() noexcept + { + return util::log::IdentificationTable::GetPtr(); + } + util::log::Level ParseLogLevel_(const std::string& logLevel) { if (logLevel.empty()) { @@ -165,21 +169,15 @@ namespace pmon::test } auto& linkState = GetLogLinkState_(); - LoggingSingletons gettersCopy{}; + LoggingSingletons getters{}; { std::lock_guard lock{ linkState.mtx }; - if (!linkState.linked) { - linkState.getters = pmLinkLogging_( - pChannel, - []() -> util::log::IdentificationTable& { - return util::log::IdentificationTable::Get_(); }); - linkState.linked = true; - } - gettersCopy = linkState.getters; + getters = pmLinkLoggingPtrs_(pChannel.get(), GetLinkIdTablePtr_()); + linkState.linked = true; } - if (gettersCopy) { - auto& dllPolicy = gettersCopy.getGlobalPolicy(); + if (getters) { + auto& dllPolicy = getters.getGlobalPolicy(); dllPolicy.SetLogLevel(level); dllPolicy.SetTraceLevel(util::log::Level::Error); dllPolicy.SetExceptionTrace(false); @@ -204,7 +202,6 @@ namespace pmon::test util::log::FlushEntryPoint(); auto& linkState = GetLogLinkState_(); std::lock_guard lock{ linkState.mtx }; - linkState.getters = {}; linkState.linked = false; } } diff --git a/IntelPresentMon/PresentMonMiddleware/LogSetup.cpp b/IntelPresentMon/PresentMonMiddleware/LogSetup.cpp index 6cbc78895..03737b6bd 100644 --- a/IntelPresentMon/PresentMonMiddleware/LogSetup.cpp +++ b/IntelPresentMon/PresentMonMiddleware/LogSetup.cpp @@ -37,9 +37,12 @@ namespace pmon::util::log // creates a copy channel to copy entries to a channel in the same process (useful when the channel // is in a different module/heap) - std::shared_ptr MakeCopyChannel_(std::shared_ptr pCopyTargetChannel) noexcept + std::shared_ptr MakeCopyChannel_(IChannel* pCopyTargetChannel) noexcept { try { + if (!pCopyTargetChannel) { + return {}; + } // channel auto pChannel = std::make_shared(); // error resolver @@ -47,7 +50,7 @@ namespace pmon::util::log // make and add the line-tracking policy pChannel->AttachComponent(std::make_shared()); // configure drivers - pChannel->AttachComponent(std::make_shared(std::move(pCopyTargetChannel))); + pChannel->AttachComponent(std::make_shared(pCopyTargetChannel), "drv:copy"); return pChannel; } catch (...) { @@ -123,12 +126,23 @@ namespace pmon::util::log return GetDefaultChannelWithFactory(MakeNullChannel_); } - void SetupCopyChannel(std::shared_ptr pCopyTargetChannel) noexcept + void SetupCopyChannel(IChannel* pCopyTargetChannel) noexcept { // reset logging level when channel is explicitly requested GlobalPolicy::Get().SetLogLevelDefault(); GlobalPolicy::Get().SetTraceLevelDefault(); - InjectDefaultChannel(MakeCopyChannel_(std::move(pCopyTargetChannel))); + SeverCopyLoggingBridge(); + InjectDefaultChannel(MakeCopyChannel_(pCopyTargetChannel)); + } + + void SeverCopyLoggingBridge() noexcept + { + try { + if (auto pChan = GetDefaultChannel()) { + pChan->AttachComponent({}, "drv:copy"); + } + } + catch (...) {} } void SetupODSChannel(Level logLevel, Level stackTraceLevel, bool exceptionTrace) noexcept @@ -136,6 +150,7 @@ namespace pmon::util::log GlobalPolicy::Get().SetLogLevel(logLevel); GlobalPolicy::Get().SetTraceLevel(stackTraceLevel); GlobalPolicy::Get().SetExceptionTrace(exceptionTrace); + SeverCopyLoggingBridge(); InjectDefaultChannel(MakeODSChannel_()); } @@ -146,6 +161,7 @@ namespace pmon::util::log try { pDiagnostics_ = std::make_shared(pConfig); // attach to existing channel if present + SeverCopyLoggingBridge(); InjectDefaultChannel(MakeDiagnosticChannel_(pDiagnostics_, pConfig->enableSynchronousLogging)); // set global logging policy based on the configuration GlobalPolicy::Get().SetLogLevel((Level)pConfig->filterLevel); @@ -165,6 +181,7 @@ namespace pmon::util::log GlobalPolicy::Get().SetLogLevel(logLevel); GlobalPolicy::Get().SetTraceLevel(stackTraceLevel); GlobalPolicy::Get().SetExceptionTrace(exceptionTrace); + SeverCopyLoggingBridge(); InjectDefaultChannel(MakeFileChannel_(std::move(path))); } diff --git a/IntelPresentMon/PresentMonMiddleware/LogSetup.h b/IntelPresentMon/PresentMonMiddleware/LogSetup.h index 6f6be59af..dd2115290 100644 --- a/IntelPresentMon/PresentMonMiddleware/LogSetup.h +++ b/IntelPresentMon/PresentMonMiddleware/LogSetup.h @@ -6,7 +6,9 @@ namespace pmon::util::log { - void SetupCopyChannel(std::shared_ptr pCopyTargetChannel) noexcept; + void SetupCopyChannel(IChannel* pCopyTargetChannel) noexcept; + // drop cross-module copy bridge before destroying middleware log channel + void SeverCopyLoggingBridge() noexcept; void SetupODSChannel(Level logLevel, Level stackTraceLevel, bool exceptionTrace) noexcept; void SetupDiagnosticChannel(const PM_DIAGNOSTIC_CONFIGURATION* pConfig) noexcept; void SetupFileChannel(std::filesystem::path path, Level logLevel, Level stackTraceLevel, diff --git a/IntelPresentMon/SampleClient/LogSetup.cpp b/IntelPresentMon/SampleClient/LogSetup.cpp index 06e98b049..b93cf0597 100644 --- a/IntelPresentMon/SampleClient/LogSetup.cpp +++ b/IntelPresentMon/SampleClient/LogSetup.cpp @@ -64,8 +64,7 @@ namespace p2sam // get the channel to work on it auto pChan = GetDefaultChannel(); // connect dll channel and id table to exe, get access to global settings in dll - const auto getters = pmLinkLogging_(pChan, []() -> IdentificationTable& { - return IdentificationTable::Get_(); }); + const auto getters = pmLinkLoggingPtrs_(pChan.get(), IdentificationTable::GetPtr()); // shortcut for command line const auto& opt = clio::Options::Get(); // configure logging based on command line From 63ecc77c1562e4b2dda2192d6d5002b6134b60dd Mon Sep 17 00:00:00 2001 From: "Taylor, Raymond" Date: Thu, 16 Jul 2026 17:50:29 -0700 Subject: [PATCH 2/4] fixing test hangs in etl tests --- .../PresentMonAPI2Tests/EtlTests.cpp | 35 ++++++++++-- .../PresentMonAPI2Tests/TestProcess.h | 9 ++- .../PresentMonMiddleware/Middleware.cpp | 22 ++++++- .../PresentMonMiddleware/Middleware.h | 2 +- .../PresentMonService/FrameBroadcaster.h | 11 +++- .../MockPresentMonSession.cpp | 57 ++++++++++++------- .../PresentMonService/MockPresentMonSession.h | 4 +- 7 files changed, 107 insertions(+), 33 deletions(-) diff --git a/IntelPresentMon/PresentMonAPI2Tests/EtlTests.cpp b/IntelPresentMon/PresentMonAPI2Tests/EtlTests.cpp index ebfd1e4d5..d649aaa3c 100644 --- a/IntelPresentMon/PresentMonAPI2Tests/EtlTests.cpp +++ b/IntelPresentMon/PresentMonAPI2Tests/EtlTests.cpp @@ -3,7 +3,6 @@ #include "../CommonUtilities/win/WinAPI.h" #include #include "CppUnitTest.h" -#include "FirstFrameWait.h" #include "Folders.h" #include "StatusComparison.h" #include "TestProcess.h" @@ -205,11 +204,18 @@ namespace EtlTests processTracker = pSession->TrackProcess(processId, true, true); - if (!pmon::tests::TryWaitForFirstFrame( - controlPipe_, - processId, - "etl-playback", - std::chrono::seconds(waitTimeSecs))) { + const auto firstFrameDeadline = std::chrono::steady_clock::now() + std::chrono::seconds(waitTimeSecs); + bool gotFirstFrame = false; + while (std::chrono::steady_clock::now() < firstFrameDeadline) { + frameQuery.Consume(processTracker, blobs); + if (blobs.GetNumBlobsPopulated() > 0) { + gotFirstFrame = true; + goldCsvFile.VerifyBlobAgainstCsv(processName, processId, queryElements, blobs, outputCsvFile); + break; + } + std::this_thread::sleep_for(8ms); + } + if (!gotFirstFrame) { throw CsvException("Timeout waiting to consume first frame"); } @@ -228,6 +234,23 @@ namespace EtlTests goldCsvFile.VerifyBlobAgainstCsv(processName, processId, queryElements, blobs, outputCsvFile); } } + // Drain the backpressured playback ring so the service output thread cannot + // block indefinitely during session teardown. + int drainEmptyPollCount = 0; + while (drainEmptyPollCount < pollCount) { + frameQuery.Consume(processTracker, blobs); + if (blobs.GetNumBlobsPopulated() == 0) { + if (++drainEmptyPollCount >= pollCount) { + break; + } + std::this_thread::sleep_for(8ms); + } + else { + drainEmptyPollCount = 0; + goldCsvFile.VerifyBlobAgainstCsv(processName, processId, queryElements, blobs, outputCsvFile); + } + } + processTracker.FlushFrames(); } TEST_CLASS(GoldEtlCsvTests) diff --git a/IntelPresentMon/PresentMonAPI2Tests/TestProcess.h b/IntelPresentMon/PresentMonAPI2Tests/TestProcess.h index f10d71dbc..299db5028 100644 --- a/IntelPresentMon/PresentMonAPI2Tests/TestProcess.h +++ b/IntelPresentMon/PresentMonAPI2Tests/TestProcess.h @@ -183,7 +183,14 @@ class ConnectedTestProcess : public TestProcess { Assert::IsTrue(process_.running()); Assert::AreEqual("quit-ok"s, Command("quit")); - process_.wait(); + static constexpr auto kServiceExitTimeout = 15s; + if (!WaitForExit(kServiceExitTimeout)) { + Logger::WriteMessage(std::format( + "Service did not exit within {}ms after quit; terminating\n", + kServiceExitTimeout.count()).c_str()); + Murder(); + return; + } } void Ping() { diff --git a/IntelPresentMon/PresentMonMiddleware/Middleware.cpp b/IntelPresentMon/PresentMonMiddleware/Middleware.cpp index f6e1598c8..8a1ccd040 100644 --- a/IntelPresentMon/PresentMonMiddleware/Middleware.cpp +++ b/IntelPresentMon/PresentMonMiddleware/Middleware.cpp @@ -137,7 +137,27 @@ namespace pmon::mid Middleware& Middleware::operator=(Middleware&&) = default; - Middleware::~Middleware() = default; + Middleware::~Middleware() noexcept + { + try { + std::vector trackedPids; + trackedPids.reserve(frameMetricsSources_.size()); + for (const auto& entry : frameMetricsSources_) { + trackedPids.push_back(entry.first); + } + for (uint32_t pid : trackedPids) { + try { + StopTracking(pid); + } + catch (...) { + pmlog_warn(std::format("StopTracking failed during middleware teardown for pid [{}]", pid)); + } + } + } + catch (...) { + pmlog_error(util::ReportException("Middleware teardown")); + } + } const PM_INTROSPECTION_ROOT* Middleware::GetIntrospectionData() { diff --git a/IntelPresentMon/PresentMonMiddleware/Middleware.h b/IntelPresentMon/PresentMonMiddleware/Middleware.h index 532c7aabd..5083bba8e 100644 --- a/IntelPresentMon/PresentMonMiddleware/Middleware.h +++ b/IntelPresentMon/PresentMonMiddleware/Middleware.h @@ -28,7 +28,7 @@ namespace pmon::mid Middleware& operator=(const Middleware&) = delete; Middleware(Middleware&&); Middleware& operator=(Middleware&&); - ~Middleware(); + ~Middleware() noexcept; const PM_INTROSPECTION_ROOT* GetIntrospectionData(); void FreeIntrospectionData(const PM_INTROSPECTION_ROOT* pRoot); void StartTracking(uint32_t processId); diff --git a/IntelPresentMon/PresentMonService/FrameBroadcaster.h b/IntelPresentMon/PresentMonService/FrameBroadcaster.h index 811c041bc..3b7bbd784 100644 --- a/IntelPresentMon/PresentMonService/FrameBroadcaster.h +++ b/IntelPresentMon/PresentMonService/FrameBroadcaster.h @@ -65,9 +65,18 @@ namespace pmon::svc pSegment = comms_.GetFrameDataSegment(present.ProcessId); } if (pSegment) { - pSegment->GetStore().frameData.Push(FrameData::CopyFrameData(present)); + if (!pSegment->GetStore().frameData.Push(FrameData::CopyFrameData(present), timeoutMs)) { + pmlog_warn("Frame broadcast push timed out under backpressure") + .pmwatch(present.ProcessId); + } } } + void ReleaseAllBackpressure() + { + for (auto pid : GetPids()) { + UpdateReadSerial(pid, GetCurrentWriteSerial(pid).value_or(0)); + } + } // Update the single consumer cursor for a backpressured playback ring. Playback // backpressure is SPSC: one producer in the service and one owning client reader. void UpdateReadSerial(uint32_t pid, uint64_t effectiveSerial) diff --git a/IntelPresentMon/PresentMonService/MockPresentMonSession.cpp b/IntelPresentMon/PresentMonService/MockPresentMonSession.cpp index 732c22fcc..173b68b98 100644 --- a/IntelPresentMon/PresentMonService/MockPresentMonSession.cpp +++ b/IntelPresentMon/PresentMonService/MockPresentMonSession.cpp @@ -83,7 +83,7 @@ PM_STATUS MockPresentMonSession::UpdateTracking(const std::unordered_setReleaseAllBackpressure(); + } - // PHASE 2: Safe cleanup after threads have finished - std::lock_guard lock(session_mutex_); + trace_session_.Stop(); + stop_trace_join_pending_.store(true, std::memory_order_release); +} - if (evtStreamingStarted_) { - evtStreamingStarted_.Reset(); - } +void MockPresentMonSession::FinalizeStopTraceSession() { + if (!stop_trace_join_pending_.exchange(false, std::memory_order_acq_rel)) { + return; + } - if (pm_consumer_) { - pm_consumer_.reset(); - } - started_processes_.clear(); + WaitForConsumerThreadToExit(); + StopOutputThread(); + + std::lock_guard lock(session_mutex_); + + if (evtStreamingStarted_) { + evtStreamingStarted_.Reset(); + } + + if (pm_consumer_) { + pm_consumer_.reset(); } + started_processes_.clear(); } void MockPresentMonSession::StartConsumerThread(TRACEHANDLE traceHandle) { diff --git a/IntelPresentMon/PresentMonService/MockPresentMonSession.h b/IntelPresentMon/PresentMonService/MockPresentMonSession.h index 945686890..008b05624 100644 --- a/IntelPresentMon/PresentMonService/MockPresentMonSession.h +++ b/IntelPresentMon/PresentMonService/MockPresentMonSession.h @@ -35,7 +35,8 @@ class MockPresentMonSession : public PresentMonSession bool isPlaybackRetimed, bool isPlaybackBackpressured, bool isPlaybackResetOldest); - void StopTraceSession(); + void RequestStopTraceSession(); + void FinalizeStopTraceSession(); void DequeueAnalyzedInfo( std::vector* processEvents, @@ -83,4 +84,5 @@ class MockPresentMonSession : public PresentMonSession mutable std::mutex session_mutex_; // TODO: evaluate necessity/improvements on this construct std::atomic session_active_{false}; // Lock-free session state for hot path queries + std::atomic stop_trace_join_pending_{ false }; }; From 07098922ce98652f61dd2755aadd1b033d3495eb Mon Sep 17 00:00:00 2001 From: chili Date: Thu, 16 Jul 2026 18:31:23 -0700 Subject: [PATCH 3/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- IntelPresentMon/PresentMonAPI2Tests/TestProcess.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/IntelPresentMon/PresentMonAPI2Tests/TestProcess.h b/IntelPresentMon/PresentMonAPI2Tests/TestProcess.h index 299db5028..ec573dcbb 100644 --- a/IntelPresentMon/PresentMonAPI2Tests/TestProcess.h +++ b/IntelPresentMon/PresentMonAPI2Tests/TestProcess.h @@ -186,7 +186,7 @@ class ConnectedTestProcess : public TestProcess static constexpr auto kServiceExitTimeout = 15s; if (!WaitForExit(kServiceExitTimeout)) { Logger::WriteMessage(std::format( - "Service did not exit within {}ms after quit; terminating\n", + "Service did not exit within {}s after quit; terminating\n", kServiceExitTimeout.count()).c_str()); Murder(); return; From a13ea69a32d9442f987f6768d6f072cbd6dcfc47 Mon Sep 17 00:00:00 2001 From: chili Date: Thu, 16 Jul 2026 19:16:09 -0700 Subject: [PATCH 4/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- IntelPresentMon/PresentMonService/MockPresentMonSession.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/IntelPresentMon/PresentMonService/MockPresentMonSession.cpp b/IntelPresentMon/PresentMonService/MockPresentMonSession.cpp index 173b68b98..c043275ab 100644 --- a/IntelPresentMon/PresentMonService/MockPresentMonSession.cpp +++ b/IntelPresentMon/PresentMonService/MockPresentMonSession.cpp @@ -96,6 +96,9 @@ bool MockPresentMonSession::CheckTraceSessions(bool forceTerminate) { if (stop_trace_join_pending_.load(std::memory_order_acquire)) { FinalizeStopTraceSession(); + if (forceTerminate) { + ClearTrackedProcesses(); + } return true; }