Skip to content

Commit

Permalink
[remoting host][linux] Add a watchdog for the IgnoreXServerGrabs
Browse files Browse the repository at this point in the history
Per crbug.com/1130090, it appears that it's the call to
IgnoreXServerGrabs() that has been blocking the network thread on the
prober VMs.

This CL adds a watchdog that monitors the time it takes to call
IgnoreXServerGrabs() and crashes the host if it takes too long. This
doesn't fix the underlying problem of IgnoreXServerGrabs() being
blocky, but should be sufficient to calm down the probers, and
prove/disprove the hypothesis.

Bug: 1130090
Change-Id: I63954b4cdf72a2015344a4900d3e1ce6f9e5ae81
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2725026
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#858376}
  • Loading branch information
ywh233 authored and Chromium LUCI CQ committed Feb 27, 2021
1 parent acb6b7c commit 9fae380
Showing 1 changed file with 42 additions and 1 deletion.
43 changes: 42 additions & 1 deletion remoting/host/basic_desktop_environment.cc
Expand Up @@ -29,12 +29,46 @@
#endif

#if defined(USE_X11)
#include "base/threading/watchdog.h"
#include "remoting/host/linux/x11_util.h"
#include "ui/base/ui_base_features.h"
#endif

namespace remoting {

#if defined(USE_X11)

namespace {

// The maximum amount of time we will wait for the IgnoreXServerGrabs() to
// return before we crash the host.
constexpr base::TimeDelta kWaitForIgnoreXServerGrabsTimeout =
base::TimeDelta::FromSeconds(30);

// Helper class to monitor the call to
// webrtc::SharedXDisplay::IgnoreXServerGrabs() (on a temporary thread), which
// has been observed to occasionally hang forever and zombify the host.
// This class crashes the host if the IgnoreXServerGrabs() call takes too long,
// so that the ME2ME daemon process can respawn the host.
// See: crbug.com/1130090
class IgnoreXServerGrabsWatchdog : public base::Watchdog {
public:
IgnoreXServerGrabsWatchdog()
: base::Watchdog(kWaitForIgnoreXServerGrabsTimeout,
"IgnoreXServerGrabs Watchdog",
/* enabled= */ true) {}
~IgnoreXServerGrabsWatchdog() override = default;

void Alarm() override {
// Crash the host if IgnoreXServerGrabs() takes too long.
CHECK(false) << "IgnoreXServerGrabs() timed out.";
}
};

} // namespace

#endif // defined(USE_X11)

BasicDesktopEnvironment::~BasicDesktopEnvironment() {
DCHECK(caller_task_runner_->BelongsToCurrentThread());
}
Expand Down Expand Up @@ -131,8 +165,15 @@ BasicDesktopEnvironment::BasicDesktopEnvironment(
options_(options) {
DCHECK(caller_task_runner_->BelongsToCurrentThread());
#if defined(USE_X11)
if (!features::IsUsingOzonePlatform())
if (!features::IsUsingOzonePlatform()) {
// TODO(yuweih): The watchdog is just to test the hypothesis.
// The IgnoreXServerGrabs() call should probably be moved to whichever
// thread that created desktop_capture_options().x_display().
IgnoreXServerGrabsWatchdog watchdog;
watchdog.Arm();
desktop_capture_options().x_display()->IgnoreXServerGrabs();
watchdog.Disarm();
}
#elif defined(OS_WIN)
// The options passed to this instance are determined by a process running in
// Session 0. Access to DirectX functions in Session 0 is limited so the
Expand Down

0 comments on commit 9fae380

Please sign in to comment.