863490 - Unloading page with embedded YouTube Flash video logs "grabbing the JNIEnv from the wrong thread" errors

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Plugins, defect)

Description

[Chris Peterson [:cpeterson]]

After playing the following embedded YouTube video and then unloading the page, I hit some "grabbing the JNIEnv from the wrong thread" warnings in logcat.

http://www.qiwireless.com/nokia-md-51w-jbl-playup-unboxing-and-demo-video/

Setting a breakpoint in GetJNIEnv(), I see that Gecko is trying to release a plugin SurfaceTexture on a worker thread, not the Java UI thread.

We currently "recover" from these JNIEnv errors by returning early without doing any work.

I/AndroidBridge( 9891): ###!!!!!!! Something's grabbing the JNIEnv from the wrong thread! (thr 0x7026fc88 should be 0x40e71a90)

#0  0x7a0548f0 in mozilla::AndroidBridge::GetJNIEnv () at widget/android/AndroidBridge.h:127
#1  0x7a0a0006 in mozilla::AndroidBridge::UnregisterSurfaceTextureFrameListener (this=0x74a48170, surfaceTexture=0x1d500276) at widget/android/AndroidBridge.cpp:2314
#2  0x7a50f76a in nsSurfaceTexture::~nsSurfaceTexture (this=0x80152540, __in_chrg=<optimized out>) at gfx/thebes/nsSurfaceTexture.cpp:212
#3  0x7a0585ec in nsSurfaceTexture::Release (this=0x80152540) at ../../../dist/include/nsSurfaceTexture.h:25
#4  0x7a55cb56 in assign_assuming_AddRef (newPtr=0x0, this=0x7f99c838) at ../../dist/include/nsAutoPtr.h:868
#5  assign_with_AddRef (rawPtr=0x0, this=0x7f99c838) at ../../dist/include/nsAutoPtr.h:852
#6  operator= (rhs=0x0, this=0x7f99c838) at ../../dist/include/nsAutoPtr.h:936
#7  mozilla::gl::SurfaceTextureWrapper::~SurfaceTextureWrapper (this=0x7f99c830, __in_chrg=<optimized out>) at gfx/gl/GLContextProviderEGL.cpp:785
#8  0x7a55cb7e in mozilla::gl::SurfaceTextureWrapper::~SurfaceTextureWrapper (this=0x7f99c830, __in_chrg=<optimized out>) at gfx/gl/GLContextProviderEGL.cpp:786
#9  0x7a55c780 in mozilla::gl::GLContextEGL::ReleaseSharedHandle (this=<optimized out>, shareType=<optimized out>, sharedHandle=2140784688) at gfx/gl/GLContextProviderEGL.c
pp:998
#10 0x7a54c9a8 in mozilla::layers::SharedTextureHostOGL::DeleteTextures (this=0x7ff78700) at gfx/layers/opengl/TextureHostOGL.cpp:222
#11 0x7a54d28e in mozilla::layers::SharedTextureHostOGL::SetCompositor (this=0x7ff78700, aCompositor=0x0) at gfx/layers/opengl/TextureHostOGL.cpp:211
#12 0x7a54a584 in mozilla::layers::ImageHostSingle::SetCompositor (this=<optimized out>, aCompositor=<optimized out>) at gfx/layers/composite/ImageHost.cpp:23
#13 0x7a51de96 in Detach (this=<optimized out>) at gfx/layers/composite/CompositableHost.h:156
#14 mozilla::layers::CompositableParent::ActorDestroy (this=<optimized out>, why=<optimized out>) at gfx/layers/composite/CompositableHost.cpp:75
#15 0x7a174ff6 in mozilla::plugins::PPluginBackgroundDestroyerParent::DestroySubtree (this=0x8028fb40, why=why@entry=mozilla::ipc::IProtocolManager<mozilla::ipc::RPCChannel::RPCListener>::Deletion) at objdir-android/ipc/ipdl/PPluginBackgroundDestroyerParent.cpp:332
#16 0x7a1b06fc in OnMessageReceived (__msg=..., this=0x8028fb40) at objdir-android/ipc/ipdl/PCompositableParent.cpp:171
#17 mozilla::layers::PCompositableParent::OnMessageReceived (this=0x8028fb40, __msg=...) at objdir-android/ipc/ipdl/PCompositableParent.cpp:144
#18 0x7a1b2a32 in mozilla::layers::PCompositorParent::OnMessageReceived (this=0x780b7800, __msg=...) at objdir-android/ipc/ipdl/PCompositorParent.cpp:311
#19 0x7a156e2e in mozilla::ipc::AsyncChannel::OnDispatchMessage (this=0x780b7808, msg=...) at ipc/glue/AsyncChannel.cpp:494
#20 0x7a15dca6 in mozilla::ipc::RPCChannel::OnMaybeDequeueOne (this=0x780b7808) at ipc/glue/RPCChannel.cpp:402
#21 0x7a097f66 in DispatchToMethod<ThumbnailRunnable, tag_nsresult (ThumbnailRunnable::*)()> (method=(tag_nsresult (ThumbnailRunnable::*)(ThumbnailRunnable * const)) 0x7a15dbd5 <mozilla::ipc::RPCChannel::OnMayb
eDequeueOne()>, obj=<optimized out>, arg=...) at ipc/chromium/src/base/tuple.h:383
#22 RunnableMethod<ThumbnailRunnable, tag_nsresult (ThumbnailRunnable::*)(), Tuple0>::Run (this=<optimized out>) at ipc/chromium/src/base/task.h:307
#23 0x7a15c856 in Run (this=<optimized out>) at ../../dist/include/mozilla/ipc/RPCChannel.h:425
#24 mozilla::ipc::RPCChannel::DequeueTask::Run (this=<optimized out>) at ../../dist/include/mozilla/ipc/RPCChannel.h:448
#25 0x7a4c8b2a in MessageLoop::RunTask (this=0x785ffdec, task=0x7792c3a0) at ipc/chromium/src/base/message_loop.cc:334
#26 0x7a4c9506 in MessageLoop::DeferOrRunPendingTask (this=<optimized out>, pending_task=...) at ipc/chromium/src/base/message_loop.cc:342
#27 0x7a4ca8bc in DoWork (this=<optimized out>) at ipc/chromium/src/base/message_loop.cc:442
#28 MessageLoop::DoWork (this=0x785ffdec) at ipc/chromium/src/base/message_loop.cc:421
#29 0x7a4caaac in base::MessagePumpDefault::Run (this=0x78275440, delegate=0x785ffdec) at ipc/chromium/src/base/message_pump_default.cc:23
#30 0x7a4c8ca2 in MessageLoop::RunInternal (this=this@entry=0x785ffdec) at ipc/chromium/src/base/message_loop.cc:216
#31 0x7a4c8d00 in RunHandler (this=0x785ffdec) at ipc/chromium/src/base/message_loop.cc:209
#32 MessageLoop::Run (this=0x785ffdec) at ipc/chromium/src/base/message_loop.cc:183
#33 0x7a4d23cc in base::Thread::ThreadMain (this=0x7825b3d0) at ipc/chromium/src/base/thread.cc:156
#34 0x7a4de10e in ThreadFunc (closure=<optimized out>) at ipc/chromium/src/base/platform_thread_posix.cc:39
#35 0x402a341c in __thread_entry () from /Users/cpeterson/Code/mozilla/jimdb/lib/0172ddd29f0b445c/system/lib/libc.so
#36 0x402a2b10 in pthread_create () from /Users/cpeterson/Code/mozilla/jimdb/lib/0172ddd29f0b445c/system/lib/libc.so
#37 0x00000000 in ?? ()

Comment 1

[Chris Peterson [:cpeterson]]

Attachment: part-1-get-worker-thread-jnienv.patch

Part 1: Get the correct JNIEnv for the worker thread that is destroying the Flash plugin.

Comment 2

[Chris Peterson [:cpeterson]]

Attachment: part-2-assert-jni-thread.patch

Part 2: Assert GetJNIEnv() is called on the correct thread. This assertion only affects debug builds, though the error message is still logged in release builds.

Comment 3

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ddf65186f94
https://hg.mozilla.org/integration/mozilla-inbound/rev/936accf907a2

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/9ddf65186f94
https://hg.mozilla.org/mozilla-central/rev/936accf907a2

Comment 5

[Chris Peterson [:cpeterson]]

Comment on attachment 739340 [details] [diff] [review]
part-1-get-worker-thread-jnienv.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: This bug is not a regression, so no change from previous releases.
Testing completed (on m-c, etc.): 3 days on m-c
Risk to taking this patch (and alternatives if risky): Small risk because some Flash plugin cleanup code will run that was not previously run. However, running this cleanup code might lessen the risk from other Flash bugs, but I do not have any crash reports from the field that directly point to this (real) bug.
String or IDL/UUID changes made by this patch: N/A

Comment 6

[Alex Keybl [:akeybl]]

(In reply to Chris Peterson (:cpeterson) from comment #5)
> Risk to taking this patch (and alternatives if risky): Small risk because
> some Flash plugin cleanup code will run that was not previously run.
> However, running this cleanup code might lessen the risk from other Flash
> bugs, but I do not have any crash reports from the field that directly point
> to this (real) bug.

How do you think a plugin regression would poke its head up - other embedded Flash issues? We'd want to be on the lookout for new related regressions.

+Tyler

Comment 7

[Chris Peterson [:cpeterson]]

(In reply to Alex Keybl [:akeybl] from comment #6)
> How do you think a plugin regression would poke its head up - other embedded
> Flash issues? We'd want to be on the lookout for new related regressions.

If this change caused a Flash regression, it would probably show up as a crash when closing a page with Flash content.

Comment 8

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/17f6863a9176