Skip to content

Commit 6a99330

Browse files
planetchiliplanetchili
authored
Merge pull request #515 from GameTechDev/fix/additional-coverifty-fixes
Fix/additional coverifty fixes
2 parents 177afca + 46a7355 commit 6a99330

File tree

6 files changed

+55
-25
lines changed

6 files changed

+55
-25
lines changed

‎IntelPresentMon/PresentMonMiddleware/ConcreteMiddleware.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -884,7 +884,7 @@ static void ReportMetricsHelper(
884884
// If the simulation start time is less than the last displayed simulation start time it means
885885
// we are transitioning to app provider events.
886886
if (simStartTime > chain->mLastDisplayedSimStartTime) {
887-
metrics.mAnimationError = pmSession.TimestampDeltaToMilliSeconds(screenTime - chain->mLastDisplayedScreenTime,
887+
metrics.mAnimationError = pmSession.TimestampDeltaToMilliSeconds(screenTime - chain->mLastDisplayedAppScreenTime,
888888
simStartTime - chain->mLastDisplayedSimStartTime);
889889
chain->mAnimationError.push_back(std::abs(metrics.mAnimationError));
890890
}

‎IntelPresentMon/PresentMonMiddleware/FrameEventQuery.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1598,9 +1598,9 @@ void PM_FRAME_QUERY::Context::UpdateSourceData(const PmNsmFrameData* pSourceFram
15981598
cpuStart = pFrameDataOfLastAppPresented->present_event.PresentStartTime +
15991599
pFrameDataOfLastAppPresented->present_event.TimeInPresent;
16001600
}
1601-
if (pFrameDataOfLastPresented->present_event.PclSimStartTime != 0) {
1601+
if (pFrameDataOfLastPresented && pFrameDataOfLastPresented->present_event.PclSimStartTime != 0) {
16021602
frameTimingData.lastAppSimStartTime = pFrameDataOfLastAppPresented->present_event.PclSimStartTime;
1603-
} else if (pFrameDataOfLastPresented->present_event.AppSimStartTime != 0) {
1603+
} else if (pFrameDataOfLastPresented && pFrameDataOfLastPresented->present_event.AppSimStartTime != 0) {
16041604
frameTimingData.lastAppSimStartTime = pFrameDataOfLastAppPresented->present_event.AppSimStartTime;
16051605
}
16061606
}

‎IntelPresentMon/PresentMonService/MockPresentMonSession.cpp‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ MockPresentMonSession::MockPresentMonSession()
1515
}
1616

1717
bool MockPresentMonSession::IsTraceSessionActive() {
18-
return (pm_consumer_ != nullptr);
18+
return session_active_.load(std::memory_order_acquire);
1919
}
2020

2121
PM_STATUS MockPresentMonSession::StartStreaming(uint32_t client_process_id,
@@ -68,7 +68,7 @@ void MockPresentMonSession::StopStreaming(uint32_t client_process_id,
6868
}
6969

7070
bool MockPresentMonSession::CheckTraceSessions(bool forceTerminate) {
71-
if (pm_consumer_ && stop_playback_requested_ == true) {
71+
if (session_active_.load(std::memory_order_acquire) && stop_playback_requested_ == true) {
7272
StopTraceSession();
7373
return true;
7474
}
@@ -172,14 +172,19 @@ PM_STATUS MockPresentMonSession::StartTraceSession(uint32_t processId, const std
172172
// Set the process id for this etl session
173173
etlProcessId_ = processId;
174174

175+
// Mark session as active (atomic operation)
176+
session_active_.store(true, std::memory_order_release);
177+
175178
// Start the consumer and output threads
176179
StartConsumerThread(trace_session_.mTraceHandle);
177180
StartOutputThread();
178181
return PM_STATUS::PM_STATUS_SUCCESS;
179182
}
180183

181184
void MockPresentMonSession::StopTraceSession() {
182-
std::lock_guard<std::mutex> lock(session_mutex_);
185+
// PHASE 1: Signal shutdown and wait for threads to observe it
186+
session_active_.store(false, std::memory_order_release);
187+
183188
// Stop the trace session.
184189
trace_session_.Stop();
185190

@@ -188,6 +193,9 @@ void MockPresentMonSession::StopTraceSession() {
188193
WaitForConsumerThreadToExit();
189194
StopOutputThread();
190195

196+
// PHASE 2: Safe cleanup after threads have finished
197+
std::lock_guard<std::mutex> lock(session_mutex_);
198+
191199
// Stop all streams
192200
streamer_.StopAllStreams();
193201

@@ -209,8 +217,11 @@ void MockPresentMonSession::WaitForConsumerThreadToExit() {
209217
void MockPresentMonSession::DequeueAnalyzedInfo(
210218
std::vector<ProcessEvent>* processEvents,
211219
std::vector<std::shared_ptr<PresentEvent>>* presentEvents) {
212-
pm_consumer_->DequeueProcessEvents(*processEvents);
213-
pm_consumer_->DequeuePresentEvents(*presentEvents);
220+
// Check if session is active before accessing pm_consumer_ (atomic guard)
221+
if (session_active_.load(std::memory_order_acquire) && pm_consumer_) {
222+
pm_consumer_->DequeueProcessEvents(*processEvents);
223+
pm_consumer_->DequeuePresentEvents(*presentEvents);
224+
}
214225
}
215226

216227
void MockPresentMonSession::AddPresents(
@@ -219,10 +230,12 @@ void MockPresentMonSession::AddPresents(
219230
uint64_t stopQpc, bool* hitStopQpc) {
220231
auto i = *presentEventIndex;
221232

222-
assert(trace_session_.mStartTimestamp.QuadPart != 0);
223-
// If mStartTimestamp contains a value an etl file is being processed.
224-
// Set this value in the streamer to have the correct start time.
225-
streamer_.SetStartQpc(trace_session_.mStartTimestamp.QuadPart);
233+
// If session is active and mStartTimestamp contains a value, an etl file is being processed.
234+
// Set this value in the streamer to have the correct start time (atomic guard).
235+
if (session_active_.load(std::memory_order_acquire)) {
236+
assert(trace_session_.mStartTimestamp.QuadPart != 0);
237+
streamer_.SetStartQpc(trace_session_.mStartTimestamp.QuadPart);
238+
}
226239

227240
for (auto n = presentEvents.size(); i < n; ++i) {
228241
auto presentEvent = presentEvents[i];

‎IntelPresentMon/PresentMonService/MockPresentMonSession.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,5 @@ class MockPresentMonSession : public PresentMonSession
8585
pmon::util::win::Event evtStreamingStarted_;
8686

8787
mutable std::mutex session_mutex_;
88+
std::atomic<bool> session_active_{false}; // Lock-free session state for hot path queries
8889
};

‎IntelPresentMon/PresentMonService/RealtimePresentMonSession.cpp‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ RealtimePresentMonSession::RealtimePresentMonSession()
2121
}
2222

2323
bool RealtimePresentMonSession::IsTraceSessionActive() {
24-
return (pm_consumer_ != nullptr);
24+
return session_active_.load(std::memory_order_acquire);
2525
}
2626

2727
PM_STATUS RealtimePresentMonSession::StartStreaming(uint32_t client_process_id,
@@ -92,9 +92,12 @@ void RealtimePresentMonSession::FlushEvents()
9292
} props{};
9393
props.Wnode.BufferSize = (ULONG)sizeof(TraceProperties);
9494
props.LoggerNameOffset = offsetof(TraceProperties, mSessionName);
95-
if (ControlTraceW(trace_session_.mSessionHandle, nullptr, &props, EVENT_TRACE_CONTROL_FLUSH)) {
96-
pmlog_warn("Failed manual flush of ETW event buffer").hr();
95+
if (session_active_.load(std::memory_order_acquire)) {
96+
if (ControlTraceW(trace_session_.mSessionHandle, nullptr, &props, EVENT_TRACE_CONTROL_FLUSH)) {
97+
pmlog_warn("Failed manual flush of ETW event buffer").hr();
98+
}
9799
}
100+
98101
}
99102

100103
void RealtimePresentMonSession::ResetEtwFlushPeriod()
@@ -175,22 +178,29 @@ PM_STATUS RealtimePresentMonSession::StartTraceSession() {
175178
trace_session_.mPMConsumer->mDeferralTimeLimit = trace_session_.mTimestampFrequency.QuadPart * 2;
176179
}
177180

181+
// Mark session as active (atomic operation)
182+
session_active_.store(true, std::memory_order_release);
183+
178184
// Start the consumer and output threads
179185
StartConsumerThread(trace_session_.mTraceHandle);
180186
StartOutputThread();
181187
return PM_STATUS::PM_STATUS_SUCCESS;
182188
}
183189

184190
void RealtimePresentMonSession::StopTraceSession() {
185-
std::lock_guard<std::mutex> lock(session_mutex_);
186-
// Stop the trace session.
191+
// PHASE 1: Signal shutdown and wait for threads to observe it
192+
session_active_.store(false, std::memory_order_release);
193+
194+
// Stop the trace session to stop new events from coming in
187195
trace_session_.Stop();
188-
189-
// Wait for the consumer and output threads to end (which are using the
190-
// consumers).
196+
197+
// Wait for threads to exit their critical sections and finish
191198
WaitForConsumerThreadToExit();
192199
StopOutputThread();
193-
200+
201+
// PHASE 2: Safe cleanup after threads have finished
202+
std::lock_guard<std::mutex> lock(session_mutex_);
203+
194204
// Stop all streams
195205
streamer_.StopAllStreams();
196206
if (evtStreamingStarted_) {
@@ -215,8 +225,11 @@ void RealtimePresentMonSession::WaitForConsumerThreadToExit() {
215225
void RealtimePresentMonSession::DequeueAnalyzedInfo(
216226
std::vector<ProcessEvent>* processEvents,
217227
std::vector<std::shared_ptr<PresentEvent>>* presentEvents) {
218-
pm_consumer_->DequeueProcessEvents(*processEvents);
219-
pm_consumer_->DequeuePresentEvents(*presentEvents);
228+
// Check if session is active before accessing pm_consumer_ (atomic guard)
229+
if (session_active_.load(std::memory_order_acquire) && pm_consumer_) {
230+
pm_consumer_->DequeueProcessEvents(*processEvents);
231+
pm_consumer_->DequeuePresentEvents(*presentEvents);
232+
}
220233
}
221234

222235
void RealtimePresentMonSession::AddPresents(
@@ -225,8 +238,10 @@ void RealtimePresentMonSession::AddPresents(
225238
uint64_t stopQpc, bool* hitStopQpc) {
226239
auto i = *presentEventIndex;
227240

228-
if (trace_session_.mStartTimestamp.QuadPart != 0) {
229-
streamer_.SetStartQpc(trace_session_.mStartTimestamp.QuadPart);
241+
if (session_active_.load(std::memory_order_acquire)) {
242+
if (trace_session_.mStartTimestamp.QuadPart != 0) {
243+
streamer_.SetStartQpc(trace_session_.mStartTimestamp.QuadPart);
244+
}
230245
}
231246

232247
// logging of ETW latency

‎IntelPresentMon/PresentMonService/RealtimePresentMonSession.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,5 @@ class RealtimePresentMonSession : public PresentMonSession
7474

7575
mutable std::mutex session_mutex_;
7676
mutable std::mutex process_mutex_;
77+
std::atomic<bool> session_active_{false}; // Lock-free session state for hot path queries
7778
};

0 commit comments

Comments
 (0)