Add DesktopCaptureOption enumerate_current_process_windows to avoid hang

Enumerating windows owned by the current process on Windows has some
complications due to the GetWindowText*() APIs potentially causing a
deadlock. The APIs will send messages to the window's message loop, and
if the message loop is waiting on this operation we will enter a
deadlock.

I previously put in a mitigation for this [1] which brought the
incidence rate down by an order of magnitude, but we are still seeing
this issue fairly frequently.

So, I've added  DesktopCaptureOption enumerate_current_process_windows
which allows consumers to avoid this issue completely by ignoring
these potentially problematic windows.

By default the flag is set to true which equates with the current
behavior, consumers can set the flag to false to get the new behavior.

I've also updated all the capturers that enumerate windows on Windows
to respect the option.

[1] https://webrtc-review.googlesource.com/c/src/+/195365

Bug: chromium:1152841
Change-Id: I0e0d868957d6fbe1e607a440b3a909d005c93ccf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219380
Commit-Queue: Austin Orion <auorion@microsoft.com>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#34191}

modules/desktop_capture/cropping_window_capturer_win.cc

@@ -130,6 +130,8 @@ class CroppingWindowCapturerWin : public CroppingWindowCapturer {
  public:
   explicit CroppingWindowCapturerWin(const DesktopCaptureOptions& options)
       : CroppingWindowCapturer(options),
+        enumerate_current_process_windows_(
+            options.enumerate_current_process_windows()),
         full_screen_window_detector_(options.full_screen_window_detector()) {}
 
   void CaptureFrame() override;
@@ -148,6 +150,8 @@ class CroppingWindowCapturerWin : public CroppingWindowCapturer {
 
   WindowCaptureHelperWin window_capture_helper_;
 
+  bool enumerate_current_process_windows_;
+
   rtc::scoped_refptr<FullScreenWindowDetector> full_screen_window_detector_;
 };
 
@@ -164,7 +168,12 @@ void CroppingWindowCapturerWin::CaptureFrame() {
             // it uses responsiveness check which could lead to performance
             // issues.
             SourceList result;
-            if (!webrtc::GetWindowList(GetWindowListFlags::kNone, &result))
+            int window_list_flags =
+                enumerate_current_process_windows_
+                    ? GetWindowListFlags::kNone
+                    : GetWindowListFlags::kIgnoreCurrentProcessWindows;
+
+            if (!webrtc::GetWindowList(window_list_flags, &result))
               return false;
 
             // Filter out windows not visible on current desktop

modules/desktop_capture/desktop_capture_options.h

@@ -98,6 +98,24 @@ class RTC_EXPORT DesktopCaptureOptions {
   }
 
 #if defined(WEBRTC_WIN)
+  // Enumerating windows owned by the current process on Windows has some
+  // complications due to |GetWindowText*()| APIs potentially causing a
+  // deadlock (see the comments in the |GetWindowListHandler()| function in
+  // window_capture_utils.cc for more details on the deadlock).
+  // To avoid this issue, consumers can either ensure that the thread that runs
+  // their message loop never waits on |GetSourceList()|, or they can set this
+  // flag to false which will prevent windows running in the current process
+  // from being enumerated and included in the results. Consumers can still
+  // provide the WindowId for their own windows to |SelectSource()| and capture
+  // them.
+  bool enumerate_current_process_windows() const {
+    return enumerate_current_process_windows_;
+  }
+  void set_enumerate_current_process_windows(
+      bool enumerate_current_process_windows) {
+    enumerate_current_process_windows_ = enumerate_current_process_windows;
+  }
+
   bool allow_use_magnification_api() const {
     return allow_use_magnification_api_;
   }
@@ -158,6 +176,7 @@ class RTC_EXPORT DesktopCaptureOptions {
   rtc::scoped_refptr<FullScreenWindowDetector> full_screen_window_detector_;
 
 #if defined(WEBRTC_WIN)
+  bool enumerate_current_process_windows_ = true;
   bool allow_use_magnification_api_ = false;
   bool allow_directx_capturer_ = false;
   bool allow_cropping_window_capturer_ = false;

modules/desktop_capture/win/wgc_capturer_win.cc

@@ -57,7 +57,8 @@ std::unique_ptr<DesktopCapturer> WgcCapturerWin::CreateRawWindowCapturer(
     const DesktopCaptureOptions& options) {
   return std::make_unique<WgcCapturerWin>(
       std::make_unique<WgcWindowSourceFactory>(),
-      std::make_unique<WindowEnumerator>());
+      std::make_unique<WindowEnumerator>(
+          options.enumerate_current_process_windows()));
 }
 
 // static

modules/desktop_capture/win/wgc_capturer_win.h

@@ -38,7 +38,8 @@ class SourceEnumerator {
 
 class WindowEnumerator final : public SourceEnumerator {
  public:
-  WindowEnumerator() = default;
+  explicit WindowEnumerator(bool enumerate_current_process_windows)
+      : enumerate_current_process_windows_(enumerate_current_process_windows) {}
 
   WindowEnumerator(const WindowEnumerator&) = delete;
   WindowEnumerator& operator=(const WindowEnumerator&) = delete;
@@ -48,12 +49,13 @@ class WindowEnumerator final : public SourceEnumerator {
   bool FindAllSources(DesktopCapturer::SourceList* sources) override {
     // WGC fails to capture windows with the WS_EX_TOOLWINDOW style, so we
     // provide it as a filter to ensure windows with the style are not returned.
-    return window_capture_helper_.EnumerateCapturableWindows(sources,
+    return window_capture_helper_.EnumerateCapturableWindows(
-                                                             WS_EX_TOOLWINDOW);
+        sources, enumerate_current_process_windows_, WS_EX_TOOLWINDOW);
   }
 
  private:
   WindowCaptureHelperWin window_capture_helper_;
+  bool enumerate_current_process_windows_;
 };
 
 class ScreenEnumerator final : public SourceEnumerator {

modules/desktop_capture/win/window_capture_utils.cc

@@ -32,26 +32,21 @@ struct GetWindowListParams {
                       DesktopCapturer::SourceList* result)
       : ignore_untitled(flags & GetWindowListFlags::kIgnoreUntitled),
         ignore_unresponsive(flags & GetWindowListFlags::kIgnoreUnresponsive),
+        ignore_current_process_windows(
+            flags & GetWindowListFlags::kIgnoreCurrentProcessWindows),
         ex_style_filters(ex_style_filters),
         result(result) {}
   const bool ignore_untitled;
   const bool ignore_unresponsive;
+  const bool ignore_current_process_windows;
   const LONG ex_style_filters;
   DesktopCapturer::SourceList* const result;
 };
 
-// If a window is owned by the current process and unresponsive, then making a
+bool IsWindowOwnedByCurrentProcess(HWND hwnd) {
-// blocking call such as GetWindowText may lead to a deadlock.
-//
-// https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getwindowtexta#remarks
-bool CanSafelyMakeBlockingCalls(HWND hwnd) {
   DWORD process_id;
   GetWindowThreadProcessId(hwnd, &process_id);
-  if (process_id != GetCurrentProcessId() || IsWindowResponding(hwnd)) {
+  return process_id == GetCurrentProcessId();
-    return true;
-  }
-
-  return false;
 }
 
 BOOL CALLBACK GetWindowListHandler(HWND hwnd, LPARAM param) {
@@ -85,11 +80,26 @@ BOOL CALLBACK GetWindowListHandler(HWND hwnd, LPARAM param) {
   window.id = reinterpret_cast<WindowId>(hwnd);
 
   // GetWindowText* are potentially blocking operations if |hwnd| is
-  // owned by the current process, and can lead to a deadlock if the message
+  // owned by the current process. The APIs will send messages to the window's
-  // pump is waiting on this thread. If we've filtered out unresponsive
+  // message loop, and if the message loop is waiting on this operation we will
-  // windows, this is not a concern, but otherwise we need to check if we can
+  // enter a deadlock.
-  // safely make blocking calls.
+  // https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getwindowtexta#remarks
-  if (params->ignore_unresponsive || CanSafelyMakeBlockingCalls(hwnd)) {
+  //
+  // To help consumers avoid this, there is a DesktopCaptureOption to ignore
+  // windows owned by the current process. Consumers should either ensure that
+  // the thread running their message loop never waits on this operation, or use
+  // the option to exclude these windows from the source list.
+  bool owned_by_current_process = IsWindowOwnedByCurrentProcess(hwnd);
+  if (owned_by_current_process && params->ignore_current_process_windows) {
+    return TRUE;
+  }
+
+  // Even if consumers request to enumerate windows owned by the current
+  // process, we should not call GetWindowText* on unresponsive windows owned by
+  // the current process because we will hang. Unfortunately, we could still
+  // hang if the window becomes unresponsive after this check, hence the option
+  // to avoid these completely.
+  if (!owned_by_current_process || IsWindowResponding(hwnd)) {
     const size_t kTitleLength = 500;
     WCHAR window_title[kTitleLength] = L"";
     if (GetWindowTextLength(hwnd) != 0 &&
@@ -445,10 +455,15 @@ bool WindowCaptureHelperWin::IsWindowCloaked(HWND hwnd) {
 
 bool WindowCaptureHelperWin::EnumerateCapturableWindows(
     DesktopCapturer::SourceList* results,
+    bool enumerate_current_process_windows,
     LONG ex_style_filters) {
-  if (!webrtc::GetWindowList((GetWindowListFlags::kIgnoreUntitled |
+  int flags = (GetWindowListFlags::kIgnoreUntitled |
-                              GetWindowListFlags::kIgnoreUnresponsive),
+               GetWindowListFlags::kIgnoreUnresponsive);
-                             results, ex_style_filters)) {
+  if (!enumerate_current_process_windows) {
+    flags |= GetWindowListFlags::kIgnoreCurrentProcessWindows;
+  }
+
+  if (!webrtc::GetWindowList(flags, results, ex_style_filters)) {
     return false;
   }
 

modules/desktop_capture/win/window_capture_utils.h

@@ -78,6 +78,7 @@ enum GetWindowListFlags {
   kNone = 0x00,
   kIgnoreUntitled = 1 << 0,
   kIgnoreUnresponsive = 1 << 1,
+  kIgnoreCurrentProcessWindows = 1 << 2,
 };
 
 // Retrieves the list of top-level windows on the screen.
@@ -85,7 +86,8 @@ enum GetWindowListFlags {
 // - Those that are invisible or minimized.
 // - Program Manager & Start menu.
 // - [with kIgnoreUntitled] windows with no title.
-// - [with kIgnoreUnresponsive] windows that unresponsive.
+// - [with kIgnoreUnresponsive] windows that are unresponsive.
+// - [with kIgnoreCurrentProcessWindows] windows owned by the current process.
 // - Any windows with extended styles that match |ex_style_filters|.
 // Returns false if native APIs failed.
 bool GetWindowList(int flags,
@@ -115,6 +117,7 @@ class WindowCaptureHelperWin {
   // extended window styles (e.g. WS_EX_TOOLWINDOW) and prevent windows that
   // match from being included in |results|.
   bool EnumerateCapturableWindows(DesktopCapturer::SourceList* results,
+                                  bool enumerate_current_process_windows,
                                   LONG ex_style_filters = 0);
 
  private:

modules/desktop_capture/win/window_capture_utils_unittest.cc

@@ -137,4 +137,18 @@ TEST(WindowCaptureUtilsTest, IgnoreUntitledWindows) {
   DestroyTestWindow(info);
 }
 
+TEST(WindowCaptureUtilsTest, IgnoreCurrentProcessWindows) {
+  WindowInfo info = CreateTestWindow(kWindowTitle);
+  DesktopCapturer::SourceList window_list;
+  ASSERT_TRUE(GetWindowList(GetWindowListFlags::kIgnoreCurrentProcessWindows,
+                            &window_list));
+  EXPECT_EQ(std::find_if(window_list.begin(), window_list.end(),
+                         [&info](DesktopCapturer::Source window) {
+                           return reinterpret_cast<HWND>(window.id) ==
+                                  info.hwnd;
+                         }),
+            window_list.end());
+  DestroyTestWindow(info);
+}
+
 }  // namespace webrtc

modules/desktop_capture/win/window_capturer_win_gdi.cc

@@ -95,11 +95,14 @@ BOOL CALLBACK OwnedWindowCollector(HWND hwnd, LPARAM param) {
   return TRUE;
 }
 
-WindowCapturerWinGdi::WindowCapturerWinGdi() {}
+WindowCapturerWinGdi::WindowCapturerWinGdi(
+    bool enumerate_current_process_windows)
+    : enumerate_current_process_windows_(enumerate_current_process_windows) {}
 WindowCapturerWinGdi::~WindowCapturerWinGdi() {}
 
 bool WindowCapturerWinGdi::GetSourceList(SourceList* sources) {
-  if (!window_capture_helper_.EnumerateCapturableWindows(sources))
+  if (!window_capture_helper_.EnumerateCapturableWindows(
+          sources, enumerate_current_process_windows_))
     return false;
 
   std::map<HWND, DesktopSize> new_map;
@@ -350,7 +353,8 @@ WindowCapturerWinGdi::CaptureResults WindowCapturerWinGdi::CaptureFrame(
 
       if (!owned_windows_.empty()) {
         if (!owned_window_capturer_) {
-          owned_window_capturer_ = std::make_unique<WindowCapturerWinGdi>();
+          owned_window_capturer_ = std::make_unique<WindowCapturerWinGdi>(
+              enumerate_current_process_windows_);
         }
 
         // Owned windows are stored in top-down z-order, so this iterates in
@@ -389,7 +393,8 @@ WindowCapturerWinGdi::CaptureResults WindowCapturerWinGdi::CaptureFrame(
 // static
 std::unique_ptr<DesktopCapturer> WindowCapturerWinGdi::CreateRawWindowCapturer(
     const DesktopCaptureOptions& options) {
-  return std::unique_ptr<DesktopCapturer>(new WindowCapturerWinGdi());
+  return std::unique_ptr<DesktopCapturer>(
+      new WindowCapturerWinGdi(options.enumerate_current_process_windows()));
 }
 
 }  // namespace webrtc

modules/desktop_capture/win/window_capturer_win_gdi.h

@@ -24,7 +24,7 @@ namespace webrtc {
 
 class WindowCapturerWinGdi : public DesktopCapturer {
  public:
-  WindowCapturerWinGdi();
+  explicit WindowCapturerWinGdi(bool enumerate_current_process_windows);
 
   // Disallow copy and assign
   WindowCapturerWinGdi(const WindowCapturerWinGdi&) = delete;
@@ -61,6 +61,8 @@ class WindowCapturerWinGdi : public DesktopCapturer {
 
   WindowCaptureHelperWin window_capture_helper_;
 
+  bool enumerate_current_process_windows_;
+
   // This map is used to avoid flickering for the case when SelectWindow() calls
   // are interleaved with Capture() calls.
   std::map<HWND, DesktopSize> window_size_map_;