Skip to content

Commit 70ccd9e

Browse files
markgalvan-intelmarkgalvan-intel
committed
Additional Coverity Fixes
1 parent e851bea commit 70ccd9e

File tree

5 files changed

+54
-24
lines changed

5 files changed

+54
-24
lines changed

‎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
@@ -18,7 +18,7 @@ MockPresentMonSession::MockPresentMonSession()
1818
}
1919

2020
bool MockPresentMonSession::IsTraceSessionActive() {
21-
return (pm_consumer_ != nullptr);
21+
return session_active_.load(std::memory_order_acquire);
2222
}
2323

2424
PM_STATUS MockPresentMonSession::StartStreaming(uint32_t client_process_id,
@@ -71,7 +71,7 @@ void MockPresentMonSession::StopStreaming(uint32_t client_process_id,
7171
}
7272

7373
bool MockPresentMonSession::CheckTraceSessions(bool forceTerminate) {
74-
if (pm_consumer_ && stop_playback_requested_ == true) {
74+
if (session_active_.load(std::memory_order_acquire) && stop_playback_requested_ == true) {
7575
StopTraceSession();
7676
return true;
7777
}
@@ -170,14 +170,19 @@ PM_STATUS MockPresentMonSession::StartTraceSession(uint32_t processId, const std
170170
// Set the process id for this etl session
171171
etlProcessId_ = processId;
172172

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

179182
void MockPresentMonSession::StopTraceSession() {
180-
std::lock_guard<std::mutex> lock(session_mutex_);
183+
// PHASE 1: Signal shutdown and wait for threads to observe it
184+
session_active_.store(false, std::memory_order_release);
185+
181186
// Stop the trace session.
182187
trace_session_.Stop();
183188

@@ -186,6 +191,9 @@ void MockPresentMonSession::StopTraceSession() {
186191
WaitForConsumerThreadToExit();
187192
StopOutputThread();
188193

194+
// PHASE 2: Safe cleanup after threads have finished
195+
std::lock_guard<std::mutex> lock(session_mutex_);
196+
189197
// Stop all streams
190198
streamer_.StopAllStreams();
191199

@@ -207,8 +215,11 @@ void MockPresentMonSession::WaitForConsumerThreadToExit() {
207215
void MockPresentMonSession::DequeueAnalyzedInfo(
208216
std::vector<ProcessEvent>* processEvents,
209217
std::vector<std::shared_ptr<PresentEvent>>* presentEvents) {
210-
pm_consumer_->DequeueProcessEvents(*processEvents);
211-
pm_consumer_->DequeuePresentEvents(*presentEvents);
218+
// Check if session is active before accessing pm_consumer_ (atomic guard)
219+
if (session_active_.load(std::memory_order_acquire) && pm_consumer_) {
220+
pm_consumer_->DequeueProcessEvents(*processEvents);
221+
pm_consumer_->DequeuePresentEvents(*presentEvents);
222+
}
212223
}
213224

214225
void MockPresentMonSession::AddPresents(
@@ -217,10 +228,12 @@ void MockPresentMonSession::AddPresents(
217228
uint64_t stopQpc, bool* hitStopQpc) {
218229
auto i = *presentEventIndex;
219230

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

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

‎IntelPresentMon/PresentMonService/MockPresentMonSession.h‎

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

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

‎IntelPresentMon/PresentMonService/RealtimePresentMonSession.cpp‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ RealtimePresentMonSession::RealtimePresentMonSession()
2525
}
2626

2727
bool RealtimePresentMonSession::IsTraceSessionActive() {
28-
return (pm_consumer_ != nullptr);
28+
return session_active_.load(std::memory_order_acquire);
2929
}
3030

3131
PM_STATUS RealtimePresentMonSession::StartStreaming(uint32_t client_process_id,
@@ -96,9 +96,12 @@ void RealtimePresentMonSession::FlushEvents()
9696
} props{};
9797
props.Wnode.BufferSize = (ULONG)sizeof(TraceProperties);
9898
props.LoggerNameOffset = offsetof(TraceProperties, mSessionName);
99-
if (ControlTraceW(trace_session_.mSessionHandle, nullptr, &props, EVENT_TRACE_CONTROL_FLUSH)) {
100-
pmlog_warn("Failed manual flush of ETW event buffer").hr();
99+
if (session_active_.load(std::memory_order_acquire)) {
100+
if (ControlTraceW(trace_session_.mSessionHandle, nullptr, &props, EVENT_TRACE_CONTROL_FLUSH)) {
101+
pmlog_warn("Failed manual flush of ETW event buffer").hr();
102+
}
101103
}
104+
102105
}
103106

104107
PM_STATUS RealtimePresentMonSession::StartTraceSession() {
@@ -174,22 +177,29 @@ PM_STATUS RealtimePresentMonSession::StartTraceSession() {
174177
trace_session_.mPMConsumer->mDeferralTimeLimit = trace_session_.mTimestampFrequency.QuadPart * 2;
175178
}
176179

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

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

221234
void RealtimePresentMonSession::AddPresents(
@@ -224,8 +237,10 @@ void RealtimePresentMonSession::AddPresents(
224237
uint64_t stopQpc, bool* hitStopQpc) {
225238
auto i = *presentEventIndex;
226239

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

231246
// logging of ETW latency

‎IntelPresentMon/PresentMonService/RealtimePresentMonSession.h‎

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

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

0 commit comments

Comments
 (0)