Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(39)

Unified Diff: content/gpu/gpu_watchdog_thread.cc

Issue 836473003: Avoid gpu watchdog crash on timeout if X is un-responsive. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: review comments addressed. Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « content/gpu/gpu_watchdog_thread.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/gpu/gpu_watchdog_thread.cc
diff --git a/content/gpu/gpu_watchdog_thread.cc b/content/gpu/gpu_watchdog_thread.cc
index 02b6bff8040e8f6c7c464f309c31761da0f508f9..da3cd1b93a647982de13750cea897c040c7514ce 100644
--- a/content/gpu/gpu_watchdog_thread.cc
+++ b/content/gpu/gpu_watchdog_thread.cc
@@ -28,6 +28,10 @@ const base::FilePath::CharType
#endif
} // namespace
+#if defined(USE_X11)
+const unsigned char text[20] = "check";
+#endif
piman 2015/02/13 22:28:38 nit: move into anonymous namespace above.
+
GpuWatchdogThread::GpuWatchdogThread(int timeout)
: base::Thread("Watchdog"),
watched_message_loop_(base::MessageLoop::current()),
@@ -39,6 +43,11 @@ GpuWatchdogThread::GpuWatchdogThread(int timeout)
#endif
task_observer_(this),
suspended_(false),
+#if defined(USE_X11)
+ display_(NULL),
+ window_(0),
+ atom_(None),
+#endif
weak_factory_(this) {
DCHECK(timeout >= 0);
@@ -59,6 +68,9 @@ GpuWatchdogThread::GpuWatchdogThread(int timeout)
#if defined(OS_CHROMEOS)
tty_file_ = base::OpenFile(base::FilePath(kTtyFilePath), "r");
#endif
+#if defined(USE_X11)
+ SetupXServer();
+#endif
watched_message_loop_->AddTaskObserver(&task_observer_);
}
@@ -123,6 +135,11 @@ GpuWatchdogThread::~GpuWatchdogThread() {
fclose(tty_file_);
#endif
+#if defined(USE_X11)
+ XDestroyWindow(display_, window_);
+ XCloseDisplay(display_);
+#endif
+
watched_message_loop_->RemoveTaskObserver(&task_observer_);
}
@@ -188,9 +205,8 @@ void GpuWatchdogThread::OnCheck(bool after_suspend) {
// not respond in time.
message_loop()->PostDelayedTask(
FROM_HERE,
- base::Bind(
- &GpuWatchdogThread::DeliberatelyTerminateToRecoverFromHang,
- weak_factory_.GetWeakPtr()),
+ base::Bind(&GpuWatchdogThread::DeliberatelyTerminateToRecoverFromHang,
+ weak_factory_.GetWeakPtr()),
timeout);
}
@@ -224,6 +240,61 @@ void GpuWatchdogThread::DeliberatelyTerminateToRecoverFromHang() {
return;
}
+#if defined(USE_X11)
+ XWindowAttributes attributes;
+ XGetWindowAttributes(display_, window_, &attributes);
+
+ XSelectInput(display_, window_, PropertyChangeMask);
+ SetupXChangeProp();
+
+ XFlush(display_);
+
+ XEvent event_return;
+ base::TimeTicks deadline = base::TimeTicks::Now() + timeout_;
+ while (true) {
+ base::TimeDelta delta = deadline - base::TimeTicks::Now();
+ if (delta < base::TimeDelta()) {
+ // timeout
piman 2015/02/13 22:28:38 It would be more useful to have a comment describi
sohanjg 2015/02/16 09:44:49 Done.
+ return;
+ } else {
+ if (XCheckWindowEvent(display_, window_, PropertyChangeMask,
+ &event_return)) {
+ // got property
piman 2015/02/13 22:28:38 nit: remove comment
sohanjg 2015/02/16 09:44:49 Done.
+ if (MatchXWindowProp())
+ break;
+ }
+ struct pollfd fds[1];
+ fds[0].fd = XConnectionNumber(display_);
+ fds[0].events = POLLIN;
+ int status = poll(fds, 1, delta.InMilliseconds());
+ if (status == -1) {
+ if (errno == EINTR)
+ // restart
piman 2015/02/13 22:28:38 nit: remove comment
sohanjg 2015/02/16 09:44:49 Done.
+ continue;
+ else
+ // error, abort
piman 2015/02/13 22:28:38 nit: remove comment
sohanjg 2015/02/16 09:44:49 Done.
+ break;
piman 2015/02/13 22:28:38 In this case, we should LOG(FATAL) << "Lost X conn
sohanjg 2015/02/16 09:44:49 Done.
+ } else if (status == 0) {
+ // timeout
piman 2015/02/13 22:28:38 nit: remove comment
+ return;
+ } else {
+ while (true) {
piman 2015/02/13 22:28:38 I don't think this second block makes sense. Can w
sohanjg 2015/02/16 09:44:49 Done.
+ base::TimeDelta delta2 = deadline - base::TimeTicks::Now();
+ if (delta2 < base::TimeDelta())
+ // timeout
+ return;
+ if (XCheckWindowEvent(display_, window_, PropertyChangeMask,
+ &event_return)) {
+ if (MatchXWindowProp())
+ break;
+ } else
+ continue;
+ }
+ }
+ }
+ }
+#endif
+
// For minimal developer annoyance, don't keep terminating. You need to skip
// the call to base::Process::Terminate below in a debugger for this to be
// useful.
@@ -259,6 +330,36 @@ void GpuWatchdogThread::DeliberatelyTerminateToRecoverFromHang() {
terminated = true;
}
+#if defined(USE_X11)
+void GpuWatchdogThread::SetupXServer() {
+ display_ = XOpenDisplay(NULL);
+ window_ = XCreateWindow(display_, DefaultRootWindow(display_), 0, 0, 1, 1, 0,
+ CopyFromParent, InputOutput, CopyFromParent, 0, NULL);
+ atom_ = XInternAtom(display_, "CHECK", False);
+}
+
+void GpuWatchdogThread::SetupXChangeProp() {
+ XChangeProperty(display_, window_, atom_, XA_STRING, 8, PropModeReplace, text,
+ strlen((char*)text));
piman 2015/02/13 22:28:38 nit: no c-style cast. You should be able to use ar
sohanjg 2015/02/16 09:44:49 Done.
+}
+
+bool GpuWatchdogThread::MatchXWindowProp() {
+ unsigned long nitems = 0;
+ unsigned long nbytes = 0;
+ XAtom prop_type = None;
+ int prop_format = 0;
+ unsigned char* property_data = NULL;
+ if (XGetWindowProperty(display_, window_, atom_, 0, 65535, False,
piman 2015/02/13 22:28:38 I was more imagining that we'd check the X event,
sohanjg 2015/02/16 09:44:49 Done.
+ AnyPropertyType, &prop_type, &prop_format, &nitems,
+ &nbytes, &property_data) == Success) {
+ if (prop_type == XA_STRING && prop_format == 8 &&
+ nitems == strlen((char*)text))
+ return true;
+ }
+ return false;
+}
+
+#endif
void GpuWatchdogThread::AddPowerObserver() {
message_loop()->PostTask(
FROM_HERE,
« no previous file with comments | « content/gpu/gpu_watchdog_thread.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698