Additions to support crazy linker callbacks from the UI thread.

Crazy linker fix for

https://code.google.com/p/chromium/issues/detail?id=355595

There is a race between the crazy linker running on the native thread and the system linker running at the same time on the main UI thread,  It can show up in several places, but in its simplest incarnation appears as:

 --- crazy linker on native thread
 mprotect page writable
 --- system linker on main thread
 mprotect page writable
 write it
 mprotect page readonly
 --- crazy linker on native thread
 write it                [ <- sigsegv, page readonly (code=2) ]
 mprotect page readonly  [ not reached ]

The patch moves the crazy linker's r_map update calls so that they execute on the main UI thread.  This ensures that the crazy linker sets these pages writable and then readonly once done synchronously with system linker activity.

Companion to:

https://googleplex-android-review.git.corp.google.com/#/c/448556/

BUG=355595
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270084

[bulach, 2014-04-15 13:58:14]

lgtm % nits, thanks!

https://codereview.chromium.org/228023005/diff/40001/base/android/java/src/or...
File base/android/java/src/org/chromium/base/library_loader/Linker.java (right):

https://codereview.chromium.org/228023005/diff/40001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/Linker.java:852: public
static void PostCallbackOnMainThread(final long opaque) {
nit: s/Post/post/

https://codereview.chromium.org/228023005/diff/40001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/Linker.java:853:
ThreadUtils.postOnUiThread(new Runnable() {
nit: java, 4 space indent, 8 for continuations

https://codereview.chromium.org/228023005/diff/40001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/Linker.java:856:
nativeReceiveCallbackFromMainThread(opaque);
nit: perhaps "runCallbackOnUiThread" ?

https://codereview.chromium.org/228023005/diff/40001/base/android/linker/link...
File base/android/linker/linker_jni.cc (right):

https://codereview.chromium.org/228023005/diff/40001/base/android/linker/link...
base/android/linker/linker_jni.cc:349: static bool PostForLaterExecution(void
(*handler)(void*), void *opaque) {
nit: s/void *opaque/void* opaque/

https://codereview.chromium.org/228023005/diff/40001/base/android/linker/link...
base/android/linker/linker_jni.cc:375: s_callback_refs.clazz,
s_callback_refs.method_id, arg);
need to clear any java exception (assuming it's safe to 'swallow' them since
there's a return value here):

if (env->ExceptionCheck() == JNI_FALSE)
  return true;
env->ExceptionDescribe();
env->ExceptionClear();
return false;

[bulach, 2014-04-15 13:59:13]

sorry, forgot to mention. you may want to have on the description something
like:

"Crazy linker fix for r_map update."

since the issue is already linked from the BUG= line down below.

[simonb1, 2014-04-17 14:45:43]

https://codereview.chromium.org/228023005/diff/40001/base/android/java/src/or...
File base/android/java/src/org/chromium/base/library_loader/Linker.java (right):

https://codereview.chromium.org/228023005/diff/40001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/Linker.java:852: public
static void PostCallbackOnMainThread(final long opaque) {
On 2014/04/15 13:58:14, bulach wrote:
> nit: s/Post/post/

Done.

https://codereview.chromium.org/228023005/diff/40001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/Linker.java:853:
ThreadUtils.postOnUiThread(new Runnable() {
On 2014/04/15 13:58:14, bulach wrote:
> nit: java, 4 space indent, 8 for continuations

Done.

https://codereview.chromium.org/228023005/diff/40001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/Linker.java:856:
nativeReceiveCallbackFromMainThread(opaque);
On 2014/04/15 13:58:14, bulach wrote:
> nit: perhaps "runCallbackOnUiThread" ?

Done.

https://codereview.chromium.org/228023005/diff/40001/base/android/linker/link...
File base/android/linker/linker_jni.cc (right):

https://codereview.chromium.org/228023005/diff/40001/base/android/linker/link...
base/android/linker/linker_jni.cc:349: static bool PostForLaterExecution(void
(*handler)(void*), void *opaque) {
On 2014/04/15 13:58:14, bulach wrote:
> nit: s/void *opaque/void* opaque/

Done.

https://codereview.chromium.org/228023005/diff/40001/base/android/linker/link...
base/android/linker/linker_jni.cc:375: s_callback_refs.clazz,
s_callback_refs.method_id, arg);
On 2014/04/15 13:58:14, bulach wrote:
> need to clear any java exception (assuming it's safe to 'swallow' them since
> there's a return value here):
> 
> if (env->ExceptionCheck() == JNI_FALSE)
>   return true;
> env->ExceptionDescribe();
> env->ExceptionClear();
> return false;
> 

Done.

[bulach, 2014-05-07 16:10:52]

still lgtm, thanks! just some nits I didn't catch before, sorry!

https://codereview.chromium.org/228023005/diff/160001/base/android/linker/lin...
File base/android/linker/linker_jni.cc (right):

https://codereview.chromium.org/228023005/diff/160001/base/android/linker/lin...
base/android/linker/linker_jni.cc:306: struct CallbackRefs_class {
nit: the name should be CallbackRefsClass. however, how about
"JavaCallbackBindings" ?

https://codereview.chromium.org/228023005/diff/160001/base/android/linker/lin...
base/android/linker/linker_jni.cc:312: clazz = linker_class;
nit: theoretically linker_class is a "localref" that the GC can swipe.. to be
safe:
clazz = env->NewGlobalRef(linker_class);

[simonb1, 2014-05-07 17:25:24]

https://codereview.chromium.org/228023005/diff/160001/base/android/linker/lin...
File base/android/linker/linker_jni.cc (right):

https://codereview.chromium.org/228023005/diff/160001/base/android/linker/lin...
base/android/linker/linker_jni.cc:306: struct CallbackRefs_class {
On 2014/05/07 16:10:53, bulach wrote:
> nit: the name should be CallbackRefsClass. however, how about
> "JavaCallbackBindings" ?

Done.

https://codereview.chromium.org/228023005/diff/160001/base/android/linker/lin...
base/android/linker/linker_jni.cc:312: clazz = linker_class;
On 2014/05/07 16:10:53, bulach wrote:
> nit: theoretically linker_class is a "localref" that the GC can swipe.. to be
> safe:
> clazz = env->NewGlobalRef(linker_class);

Done.

[simonb (inactive), 2014-05-13 11:41:09]

The CQ bit was checked by simonb@chromium.org

[commit-bot: I haz the power, 2014-05-13 11:41:41]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/simonb@chromium.org/228023005/180001

[commit-bot: I haz the power, 2014-05-13 13:26:53]

Change committed as 270084