Commit c6993a03 authored by Torne (Richard Coles)'s avatar Torne (Richard Coles) Committed by Commit Bot

android: Handle pending exceptions in MessagePump.

It's possible for there to be a pending Java exception for the current
thread when the Android version of MessagePumpForUI is called, because
the ALooper we use to schedule our message pump iterations can also have
other callbacks running on it and ALooper doesn't check for exceptions
between callbacks.

One example of this is render thread exceptions on P and earlier, which
are thrown by sending a message to the UI thread in native - if we get
unlucky and our MessagePumpForUI runs afterward in the same ALooper
polling cycle, we will crash the first time we make a JNI call,
resulting in a confusing stack trace.

Avoid this by just skipping our callback execution if there is a pending
exception set at the start of the callback. If the exception ends up not
being fatal (caught by something higher up the stack) then our callback
will be run again in the next iteration, as the fd has not been reset.

Bug: 1052830
Change-Id: Ie13ceab6c2c1b988189791546fa12a5529ef17b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2062434Reviewed-by: default avatarSami Kyöstilä <skyostil@chromium.org>
Commit-Queue: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742600}
parent 7a79fefb
...@@ -83,7 +83,8 @@ STACK_ALIGN int DelayedLooperCallback(int fd, int events, void* data) { ...@@ -83,7 +83,8 @@ STACK_ALIGN int DelayedLooperCallback(int fd, int events, void* data) {
} // namespace } // namespace
MessagePumpForUI::MessagePumpForUI() { MessagePumpForUI::MessagePumpForUI()
: env_(base::android::AttachCurrentThread()) {
// The Android native ALooper uses epoll to poll our file descriptors and wake // The Android native ALooper uses epoll to poll our file descriptors and wake
// us up. We use a simple level-triggered eventfd to signal that non-delayed // us up. We use a simple level-triggered eventfd to signal that non-delayed
// work is available, and a timerfd to signal when delayed work is ready to // work is available, and a timerfd to signal when delayed work is ready to
...@@ -121,6 +122,14 @@ MessagePumpForUI::~MessagePumpForUI() { ...@@ -121,6 +122,14 @@ MessagePumpForUI::~MessagePumpForUI() {
} }
void MessagePumpForUI::OnDelayedLooperCallback() { void MessagePumpForUI::OnDelayedLooperCallback() {
// There may be non-Chromium callbacks on the same ALooper which may have left
// a pending exception set, and ALooper does not check for this between
// callbacks. Check here, and if there's already an exception, just skip this
// iteration without clearing the fd. If the exception ends up being non-fatal
// then we'll just get called again on the next polling iteration.
if (base::android::HasException(env_))
return;
// ALooper_pollOnce may call this after Quit() if OnNonDelayedLooperCallback() // ALooper_pollOnce may call this after Quit() if OnNonDelayedLooperCallback()
// resulted in Quit() in the same round. // resulted in Quit() in the same round.
if (ShouldQuit()) if (ShouldQuit())
...@@ -159,6 +168,14 @@ void MessagePumpForUI::OnDelayedLooperCallback() { ...@@ -159,6 +168,14 @@ void MessagePumpForUI::OnDelayedLooperCallback() {
} }
void MessagePumpForUI::OnNonDelayedLooperCallback() { void MessagePumpForUI::OnNonDelayedLooperCallback() {
// There may be non-Chromium callbacks on the same ALooper which may have left
// a pending exception set, and ALooper does not check for this between
// callbacks. Check here, and if there's already an exception, just skip this
// iteration without clearing the fd. If the exception ends up being non-fatal
// then we'll just get called again on the next polling iteration.
if (base::android::HasException(env_))
return;
// ALooper_pollOnce may call this after Quit() if OnDelayedLooperCallback() // ALooper_pollOnce may call this after Quit() if OnDelayedLooperCallback()
// resulted in Quit() in the same round. // resulted in Quit() in the same round.
if (ShouldQuit()) if (ShouldQuit())
......
...@@ -97,6 +97,9 @@ class BASE_EXPORT MessagePumpForUI : public MessagePump { ...@@ -97,6 +97,9 @@ class BASE_EXPORT MessagePumpForUI : public MessagePump {
// The Android Looper for this thread. // The Android Looper for this thread.
ALooper* looper_ = nullptr; ALooper* looper_ = nullptr;
// The JNIEnv* for this thread, used to check for pending exceptions.
JNIEnv* env_;
DISALLOW_COPY_AND_ASSIGN(MessagePumpForUI); DISALLOW_COPY_AND_ASSIGN(MessagePumpForUI);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment