Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions electron/native/wgc-capture/src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,19 @@ struct CaptureControl {
std::atomic<bool> paused = false;
std::mutex mutex;
std::condition_variable cv;
// Dedicated mutex/cv pair used ONLY to signal "stop was requested" to the
// main thread. The frame-processing pipeline (writeVideoFrames) holds
// `mutex` for the full duration of each frame's GPU copy + encode call,
// which can stall for a long time on slow software encoders or flaky GPU
// drivers (e.g. hybrid-graphics laptops). If the final "wait for stop"
// below shared that same mutex, it would have to wait for the encode
// pipeline to release the lock before it could even check whether stop
// was requested — turning an encode stall into a silent, unbounded stop
// hang with no diagnostic output (see issue #115). Keeping stop signaling
// on its own uncontended mutex means the stop request is always observed
// promptly, regardless of what the frame pump is doing.
std::mutex stopMutex;
std::condition_variable stopCv;
std::chrono::steady_clock::time_point pauseStartedAt;
std::chrono::steady_clock::duration totalPausedDuration{};

Expand Down Expand Up @@ -359,6 +372,7 @@ void readCaptureCommands(CaptureControl& control, const std::function<void(bool)
if (line == "stop" || line == "q" || line == "quit") {
control.stopRequested = true;
control.cv.notify_all();
control.stopCv.notify_all();
Comment on lines 373 to +375

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Lost wakeup on stopCv due to unsynchronized state modification.

The main thread evaluates stopRequested while holding stopMutex (L866-L869). However, multiple paths in the code set stopRequested to true and notify stopCv without acquiring stopMutex. If the state change and notification occur exactly after the main thread evaluates the predicate as false but before it enters the OS wait queue, the notification is lost, and the main thread will hang indefinitely.

To prevent this race condition, any modification to stopRequested intended to wake the main thread must be protected by stopMutex. As per coding guidelines, Windows WGC native capture code is platform-fragile and must be treated as security-sensitive recorder code, requiring robust synchronization to prevent hangs.

  • electron/native/wgc-capture/src/main.cpp#L373-L375: wrap the assignment to control.stopRequested in a std::scoped_lock lock(control.stopMutex);.
  • electron/native/wgc-capture/src/main.cpp#L393-L395: wrap the assignment to control.stopRequested in a std::scoped_lock lock(control.stopMutex);.
  • electron/native/wgc-capture/src/main.cpp#L601-L604: wrap the assignments to encodeFailed and control.stopRequested in a std::scoped_lock stopLock(control.stopMutex);.
  • electron/native/wgc-capture/src/main.cpp#L673-L676: wrap the assignments to encodeFailed and control.stopRequested in a std::scoped_lock stopLock(control.stopMutex);.
  • electron/native/wgc-capture/src/main.cpp#L686-L689: wrap the assignments to encodeFailed and control.stopRequested in a std::scoped_lock stopLock(control.stopMutex);.
  • electron/native/wgc-capture/src/main.cpp#L729-L732: wrap the assignments to encodeFailed and control.stopRequested in a std::scoped_lock lock(control.stopMutex);.
📍 Affects 1 file
  • electron/native/wgc-capture/src/main.cpp#L373-L375 (this comment)
  • electron/native/wgc-capture/src/main.cpp#L393-L395
  • electron/native/wgc-capture/src/main.cpp#L601-L604
  • electron/native/wgc-capture/src/main.cpp#L673-L676
  • electron/native/wgc-capture/src/main.cpp#L686-L689
  • electron/native/wgc-capture/src/main.cpp#L729-L732
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@electron/native/wgc-capture/src/main.cpp` around lines 373 - 375, Protect
every stopRequested update that wakes the main thread with control.stopMutex to
prevent lost wakeups. In electron/native/wgc-capture/src/main.cpp, update ranges
373-375 and 393-395 with scoped locks around the assignment, ranges 601-604,
673-676, and 686-689 with scoped locks covering encodeFailed and stopRequested,
and range 729-732 with a scoped lock covering stopRequested; preserve the
existing notifications.

Source: Coding guidelines

return;
}
if (line == "pause") {
Expand All @@ -378,6 +392,7 @@ void readCaptureCommands(CaptureControl& control, const std::function<void(bool)
}
control.stopRequested = true;
control.cv.notify_all();
control.stopCv.notify_all();
}

} // namespace
Expand Down Expand Up @@ -586,6 +601,7 @@ int main(int argc, char* argv[]) {
encodeFailed = true;
control.stopRequested = true;
control.cv.notify_all();
control.stopCv.notify_all();
return;
}
}
Expand All @@ -608,7 +624,7 @@ int main(int argc, char* argv[]) {
while (!control.stopRequested && !encodeFailed) {
{
std::unique_lock lock(mutex);
control.cv.wait(lock, [&] {
control.cv.wait_for(lock, std::chrono::milliseconds(100), [&] {
return control.stopRequested.load() ||
encodeFailed.load() ||
(!control.paused.load() && latestFrameTexture);
Expand Down Expand Up @@ -657,6 +673,7 @@ int main(int argc, char* argv[]) {
encodeFailed = true;
control.stopRequested = true;
control.cv.notify_all();
control.stopCv.notify_all();
return;
}
lastWrittenWebcamSequence = latestWebcamSequence;
Expand All @@ -669,6 +686,7 @@ int main(int argc, char* argv[]) {
encodeFailed = true;
control.stopRequested = true;
control.cv.notify_all();
control.stopCv.notify_all();
return;
}
if (latestFrameTexture) {
Expand Down Expand Up @@ -711,6 +729,7 @@ int main(int argc, char* argv[]) {
encodeFailed = true;
control.stopRequested = true;
control.cv.notify_all();
control.stopCv.notify_all();
return false;
}
return true;
Expand Down Expand Up @@ -840,8 +859,12 @@ int main(int argc, char* argv[]) {
std::cout << "Recording started" << std::endl;

{
std::unique_lock lock(mutex);
control.cv.wait(lock, [&] {
// Wait on the dedicated stop mutex/cv (not the frame-processing
// `mutex`), so this check is never gated on the video writer thread
// releasing a lock it may be holding for a long time inside a slow
// or stalled encode call. See the CaptureControl::stopMutex comment.
std::unique_lock lock(control.stopMutex);
control.stopCv.wait(lock, [&] {
return control.stopRequested.load();
});
}
Expand Down
Loading