New issue
Advanced search Search tips

Issue 718929 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 592556



Sign in to add a comment

[JNI crashes] Properly throw Java Exceptions from AwContentsBackgroundThreadClient::shouldInterceptRequest.

Project Member Reported by gsennton@chromium.org, May 5 2017

Issue description

Comment 1 by torne@chromium.org, May 5 2017

This will be the trickier one since it's not on the UI thread and has no JVM to return to at present; we'll have to restructure how this background thread is created to be able to rethrow the exception here.

So while this is definitely a large cause of these crashes, it may be considerably easier to address some of the others first ;)
Just throwing this out there:
http://stackoverflow.com/a/37546343

(DetachCurrentThread() supposedly invokes the UncaughtExceptionHandler though the jni-spec doesn't list DetachCurrentThread() as one of the methods that is safe to call when there is a pending exception). 

Comment 3 by boliu@chromium.org, May 5 2017

what if rethrow the exception on the main thread?
Hmm, that might work as long as we ensure the the current thread doesn't do anything else (or we clear the Java exception)?

Comment 5 by torne@chromium.org, May 5 2017

I don't think it's a good idea to DetachCurrentThread in that way; the VM would be within its rights to assert and crash in an even less useful way in native.

I'm not sure if there's a way to rethrow the exception on the main thread without obfuscating the real cause by having the original exception object be relegated to being the cause of a new one. It may be easier, though.. but maybe not too easy, from native.

I don't think it's actually *too* crazy to just move to creating this thread in Java and just have a one-time doesn't-normally-return native call to run the native event loop, though. Just, more work than the UI thread ones.

Comment 6 by boliu@chromium.org, May 5 2017

That's the FILE thread: https://cs.chromium.org/chromium/src/android_webview/native/aw_contents_io_thread_client_impl.cc?rcl=d5207a9fd950c509050476541d9e054b4eb236b4&l=320

When I changed launcher thread to be java-based, there's a real compelling reason. I don't really buy better exception handling as a good enough reason, and have android deviate more from lucky luke project
Owner: ----
Status: Untriaged (was: Assigned)
I'm not familiar with the Lucky Luke project (just read the introduction at go/lucky-luke), why would we deviate more from the lucky luke project by having another thread be java-based? (I guess that takes away our control over scheduling by letting Android determine when to schedule tasks instead?)

Comment 8 by boliu@chromium.org, May 8 2017

see discussion on  crbug.com/701442 . Basically they don't support java looper yet.
Status: Available (was: Untriaged)
Cc: boliu@chromium.org
This continues to be a major cause of uninterpretable crashes received by app developers. I guess the plans haven't advanced any further than discussed here on the bug, though?

I don't see the issue with lucky luke or in general having this thread be java-based. The point is *not* to have the thread use an Android looper - all we would do is we would create the thread in question as a java.lang.Thread, and pass it a Runnable which simply calls nativeRunTheChromiumFileThreadForever. Under normal circumstances this JNI call would never return at all, and it would continue to execute whatever chromium event loop the thread would normally run. We'd only return in order to crash with an exception.
Though, if we can establish that it's definitely safe to call DetachCurrentThread on all the android versions we care about (and in future) to trigger the exception throwing, that would be simpler.

Comment 12 by boliu@chromium.org, Feb 12 2018

re #10 hmm, yeah that works in theory.

FILE thread no longer exists now, and is fully replaced with TaskRunner (lucky luke). I guess that means all base::Thread need be a java Thread. probably not big enough of a perf difference on start up to matter.

Sign in to add a comment