Multiple LayoutWorker-panics on Android release-build

[rzambre]

Host: OS X 10.11.5
Device: Nexus 6P

I used the following commands:

• ./mach run --android
• ./mach run --android -p -x
  I also did the same using the android_params file under /sdcard/servo/ but I'm seeing the same errors.

Urls played with:

• https://www.ehow.com
• https://www.twitter.com

Sample error message:

RustAndroidGlueStdouterr: LayoutWorker worker 6/6' panicked at 'RefCell already mutably borrowed', ../src/libcore/cell.rs:444
RustAndroidGlueStdouterr: thread 'LayoutWorker worker 1/6' panicked at 'RefCell already mutably borrowed', ../src/libcore/cell.rs:444
RustAndroidGlueStdouterr: thread 'LayoutWorker worker 3/6' panicked at 'RefCell already mutably borrowed', ../src/libcore/cell.rs:444
RustAndroidGlueStdouterr: thread 'LayoutWorker worker 4/6' panicked at 'RefCell already mutably borrowed', ../src/libcore/cell.rs:444
RustAndroidGlueStdouterr: thread 'LayoutWorker worker 5/6' panicked at 'RefCell already mutably borrowed', ../src/libcore/cell.rs:444

Sometimes just before forcibly terminating Servo on the device (through the touch interface) I get this error message:

RustAndroidGlueStdouterr: thread '' panicked at 'called Result::unwrap() on an Err value: "SendError(..)"', ../src/libcore/result.rs:785

The backtrace information spitted out with RUST_BACKTRACE=1 is incomprehensible (even with the debug build) since it is all hex values.

Output of adb logcat for ./mach run --android -p -x https://www.twitter.com: https://pastebin.mozilla.org/8882118
Output of adb logcat for ./mach run --android -p 2 https://www.twitter.com: https://pastebin.mozilla.org/8882120

Do these output messages help on what might be going wrong in the Android release-builds?

[larsbergstrom]

I'm noticing that there are 6 layout worker threads here - so this is an 8 core machine? We may just have a race condition here that hasn't been exhibited because we really don't almost ever run on machines with more than 4 cores.

Can you try it passing a different -y value and see if things work better?

Obv. we need to fix this potential race, as it's Bad News :-)

[rzambre]

Yes, the Nexus 6P is a heterogeneous 8-core machine. Hmm, it seem like the thread-panicks still exist when I specify 2 and 4 threads for layout :/

./mach run --android -y 2 https://www.twitter.com: https://pastebin.mozilla.org/8882910
./mach run --android -y 4 https://www.twitter.com: https://pastebin.mozilla.org/8882909

[rzambre]

adb logcat message of a thread-panic on the release Android build:

thread 'LayoutWorker worker 1/6' panicked at 'RefCell already mutably borrowed', ../src/libcore/cell.rs:444

ndk-gdb backtrace of the thread-panic:

Thread 10 "LayoutWorker wo" hit Breakpoint 1, 0xdce84da8 in rust_panic ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
(gdb) bt
#0 0xdce84da8 in rust_panic () from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#1 0xdce6fd68 in std::panicking::rust_panic_with_hook::h983af77c1a2e581b ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#2 0xdcea6d40 in std::panicking::begin_panic::he426e15a3766089a ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#3 0xdce71b88 in std::panicking::begin_panic_fmt::hdddb415186c241e7 ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#4 0xdcea6b38 in rust_begin_unwind () from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#5 0xdcef5474 in core::panicking::panic_fmt::hf4e16cb7f0d41a25 ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#6 0xdcef60a0 in core::panicking::panic::h907815f47e914305 ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#7 0xdce21740 in ipc_channel::platform::os::OsIpcSender::send::h91df71f044370ca8 ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#8 0xdc250388 in _$LT$ipc_channel..ipc..IpcSender$LT$T$GT$$GT$::send::hb69cea0d400ed49e ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#9 0xdc253884 in net_traits::image_cache_thread::ImageCacheThread::find_image_or_metadata::h0569e0ff3e99afe4 ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#10 0xdbf78050 in layout::context::LayoutContext::get_or_request_image_or_meta::h66375bce8ae1eda5 ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#11 0xdb51dc14 in layout::fragment::ImageFragmentInfo:🆕:h5f5bfe6d0df95353 ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#12 0xdb51d250 in _$LT$layout..construct..FlowConstructor$LT$$u27$a$C$$u20$ConcreteThreadSafeLayoutNode$GT$$GT$::build_fragment_for_block::h4d4d2a012e0f4583 () from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#13 0xdb50c378 in _$LT$layout..construct..FlowConstructor$LT$$u27$a$C$$u20$ConcreteThreadSafeLayoutNode$GT$$u20$as$u20$layout..traversal..PostorderNodeMutTraversal$LT$ConcreteThreadSafeLayoutNode$GT$$GT$::process::h2c06d4a49e01a6cc ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#14 0xdb504cb8 in _$LT$layout..traversal..RecalcStyleAndConstructFlows$LT$$u27$lc$GT$$u20$as$u20$style..traversal..DomTraversalContext$LT$N$GT$$GT$::process_postorder::h504d0eeeb5acc301 ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#15 0xdb52d91c in style::parallel::top_down_dom::h5fe5cdd1bfb658c5 ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#16 0xdb4555c4 in std::panicking::try::call::he2ef11c99a6c3115 ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#17 0xdceb49b8 in __rust_try () from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#18 0xdceb4950 in __rust_maybe_catch_panic ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
---Type to continue, or q to quit---
#19 0xdb456508 in _$LT$F$u20$as$u20$alloc..boxed..FnBox$LT$A$GT$$GT$::call_box::h1caabf1435ae8c04 ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#20 0xdcea4fc0 in std::sys:🧵:Thread:🆕:thread_start::h9c883b6d445ece46 ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/libmain.so
#21 0xf6f74884 in __pthread_start(void*) ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/system/lib/libc.so
#22 0xf6f4ef76 in __start_thread ()
from /Users/rzambre/Documents/servo/servo-android/support/android/apk/obj/local/armeabi/system/lib/libc.so
#23 0x00000000 in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

[jdm]

I was surprised to learn that the in-process ipc-channel implementation uses RefCell in OsIpcSender - https://github.com/servo/ipc-channel/blob/master/platform/inprocess/mod.rs#L139 . That being said, it's not clear to me where the sender would be mutable borrowed at the same time...

[jdm]

I wonder if the issue is that we have unsafe impl Sync for OsIpcSender { }, which is clearly incorrect in the presence of refcells.

[jdm]

@rzambre See if building with servo/ipc-channel#89 improves the situation?

[rzambre]

servo/ipc-channel#89 fixes the LayoutWorker-panics :) Closing this issue.

[antrik]

Please reopen this issue: the PR that supposedly fixes it hasn't even landed in ipc-channel yet; and when that's done, the fixed ipc-channel still needs to be actually pulled into servo...