Make script load event asynchronous for internal scripts

[tetsuharuohzeki]

Fix #4504

[hoppipolla-critic-bot]

Critic review: https://critic.hoppipolla.co.uk/r/3639

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

[tetsuharuohzeki]

backtrace:

servo$ ./mach run ./tests/content/test_createElement_script.html 
ALERT: TEST-PASS | start middle end === start middle end: 
task 'ScriptTask' panicked at 'DOMRefCell<T> already mutably borrowed', dom/bindings/cell.rs:114
stack backtrace:
   1:        0x109474392 - rt::backtrace::imp::write::h29e16aba79d01a03E2s
   2:        0x1094776bf - failure::on_fail::h3bebf80c289f1c68Fjt
   3:        0x10948b0b5 - unwind::begin_unwind_inner::hbd4dc3dac6180780t2c
   4:        0x108309ead - unwind::begin_unwind::h14416017364894375544
   5:        0x10886d774 - dom::bindings::cell::DOMRefCell<T>::borrow::h14631003864739750171
   6:        0x10886d466 - dom::bindings::cell::DOMRefCell<T>.JSTraceable::trace::h15452835897272104484
   7:        0x1087605fa - page::Page...dom..bindings..trace..JSTraceable::trace::h6463eeba43d72efaq2x
   8:        0x1087603f5 - dom::bindings::trace::Rc<T>.JSTraceable::trace::h10935231059209414147
   9:        0x1086106be - dom::window::Window...dom..bindings..trace..JSTraceable::trace::h2f7c3b309769ef7ecZt
  10:        0x1086104df - dom::bindings::codegen::Bindings::WindowBinding::_trace::__rust_abi
  11:        0x108610482 - dom::bindings::codegen::Bindings::WindowBinding::_trace::h4dd8ae65675b9885YbO
  12:        0x108aff40e - _ZN2js8GCMarker19processMarkStackTopERNS_11SliceBudgetE
  13:        0x108afca25 - _ZN2js8GCMarker14drainMarkStackERNS_11SliceBudgetE
  14:        0x108a19ebf - _ZL23IncrementalCollectSliceP9JSRuntimexN2js8gcreason6ReasonENS1_18JSGCInvocationKindE
  15:        0x108a191d9 - _ZL7GCCycleP9JSRuntimebxN2js18JSGCInvocationKindENS1_8gcreason6ReasonE
  16:        0x108a171d9 - _ZL7CollectP9JSRuntimebxN2js18JSGCInvocationKindENS1_8gcreason6ReasonE
  17:        0x1089ed905 - _ZN2js14DestroyContextEP9JSContextNS_18DestroyContextModeE
  18:        0x108912ea5 - rust::Cx.Drop::drop::h2ae4d3d7970e4094SHc
  19:        0x108428842 - js..rust..Cx::glue_drop.71882::h06cf19718454cedb
  20:        0x108428577 - rc::Rc<T>.Drop::drop::h746010098434137336
  21:        0x10842843e - alloc..rc..Rc<js..rust..Cx>::glue_drop.71879::h3414ecb92167f715
  22:        0x108538835 - page..JSPageInfo::glue_drop.75900::h2cf6a8f35d7e2caf
  23:        0x1085387f6 - core..option..Option<page..JSPageInfo>::glue_drop.75897::ha70ef93940bb55b4
  24:        0x1088e57d2 - script_task::shut_down_layout::h3f9a32f13a3d3177J0z
  25:        0x1088dcf4d - script_task::ScriptTask::handle_exit_pipeline_msg::h5423b4043f4a0ca0eiz
  26:        0x1088bbe80 - script_task::ScriptTask::handle_msgs::hb883bec3770b5920FXy
  27:        0x1088ba5d2 - script_task::ScriptTask::start::h677c27d9b511a0f6uXy
  28:        0x107ec25a0 - script_task::ScriptTask.ScriptTaskFactory::create::closure.10399
  29:        0x10900a002 - rtinstrument::instrument::h8d50695563267c2aC0e
  30:        0x107ea863b - task::spawn_named_with_send_on_failure::closure.9874
  31:        0x107ebe546 - task::TaskBuilder<S>::try_future::closure.10299
  32:        0x1090e162d - task::NativeSpawner.Spawner::spawn::closure.2481
  33:        0x10948bc0c - rust_try_inner
  34:        0x10948bbf6 - rust_try
  35:        0x109488ff7 - unwind::try::h6ff4c39531acadd85Qc
  36:        0x109488ecc - task::Task::run::h7b157ac3b133c93aS2b
  37:        0x1090e1453 - task::NativeSpawner.Spawner::spawn::closure.2405
  38:        0x10948a077 - thread::thread_start::hc61b1801c678e684Ync
  39:     0x7fff8b5242fc - _pthread_body
  40:     0x7fff8b524279 - _pthread_body
task 'ScriptTask' panicked at 'DOMRefCell<T> already borrowed', dom/bindings/cell.rs:131
stack backtrace:
   1:        0x109474392 - rt::backtrace::imp::write::h29e16aba79d01a03E2s
   2:        0x1094776bf - failure::on_fail::h3bebf80c289f1c68Fjt
   3:        0x10948b0b5 - unwind::begin_unwind_inner::hbd4dc3dac6180780t2c
   4:        0x108309ead - unwind::begin_unwind::h14416017364894375544
   5:        0x10887c9b4 - dom::bindings::cell::DOMRefCell<T>::borrow_mut::h4359449043017363560
   6:        0x10887c8d8 - page::Page::mut_js_info::hde740d384abf5f25efy
   7:        0x10888d8b6 - script_task::ScriptMemoryFailsafe<'a>.Drop::drop::hdd23ddd49c4281be2My
   8:        0x107edf3e6 - script..script_task..ScriptMemoryFailsafe<'_>::glue_drop.11251::h330e6c34bd964233
   9:        0x107ec2602 - script_task::ScriptTask.ScriptTaskFactory::create::closure.10399
  10:        0x10900a002 - rtinstrument::instrument::h8d50695563267c2aC0e
  11:        0x107ea863b - task::spawn_named_with_send_on_failure::closure.9874
  12:        0x107ebe546 - task::TaskBuilder<S>::try_future::closure.10299
  13:        0x1090e162d - task::NativeSpawner.Spawner::spawn::closure.2481
  14:        0x10948bc0c - rust_try_inner
  15:        0x10948bbf6 - rust_try
  16:        0x109488ff7 - unwind::try::h6ff4c39531acadd85Qc
  17:        0x109488ecc - task::Task::run::h7b157ac3b133c93aS2b
  18:        0x1090e1453 - task::NativeSpawner.Spawner::spawn::closure.2405
  19:        0x10948a077 - thread::thread_start::hc61b1801c678e684Ync
  20:     0x7fff8b5242fc - _pthread_body
  21:     0x7fff8b524279 - _pthread_body
task failed during unwinding. aborting.
Error running mach:

    ['run', './tests/content/test_createElement_script.html']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

CalledProcessError: Command '[u'components/servo/target/servo', './tests/content/test_createElement_script.html']' returned non-zero exit status -4

  File "/Users/tetsuharu/src/servo/python/servo/post_build_commands.py", line 71, in run
    subprocess.check_call(args, env=env)
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 540, in check_call
    raise CalledProcessError(retcode, cmd)

[tetsuharuohzeki]

As the result of using lldb, I seem this block causes the crash.

[tetsuharuohzeki]

umm, if this changset is rebased on #4532, the above crash is not caused.

[tetsuharuohzeki]

I looked into about the above crash with RUST_LOG=script=4. By the result, I suspect these happens when the crash.

1. Queue the load event (A) of 1st internal script
2. Queue the load event (B) of 2nd internal script generated by 1st script
3. Dispatch the load event of document. In this event, the event handler calls window.close().
4. window.close() queue ScriptMsg::ExitWindow (C).
5. Run the task A, distpatch the load event of 1st script. Trusted<T>.drop queue the ScriptMsg::RefcountCleanup (D)
6. Run the task B of 2nd script. Trusted<T>.drop queue the ScriptMsg::RefcountCleanup (E).
7. Run the task C. This calls ScriptTask.compositor.close(). After calling it, Constellation begin to shutdown and script task continues the event loop.
8. Run the task D.
9. Constellation interrupts ExitPipelineMsg and run it.
10. Task E will not be run. LiveDOMCollection cannot release cx.

I also tested to inster sleep(1 sec) to the 1st line of Constellationhandle_exit(). As the result of it, the crash will not happen.

[tetsuharuohzeki]

rebased!

[jdm]

r+

[jdm]

@saneyuki Why would task E not run in your explanation?

[bors-servo]

saw approval from jdm
at tetsuharuohzeki@4629b7c

[bors-servo]

merging saneyuki/servo/internal = 4629b7c into auto

[bors-servo]

saneyuki/servo/internal = 4629b7c merged ok, testing candidate = 2a9acdc

[bors-servo]

all tests pass:
success: http://build.servo.org/builders/linux1/builds/549
success: http://build.servo.org/builders/linux2/builds/547
success: http://build.servo.org/builders/android/builds/585
success: http://build.servo.org/builders/mac1/builds/766
success: http://build.servo.org/builders/mac2/builds/766

[bors-servo]

fast-forwarding master to auto = 2a9acdc

[tetsuharuohzeki]

@saneyuki Why would task E not run in your explanation?

Ah, I forgot to wrote my speculation. I suspected that Task E will not be run because ExitPipelineMsg finalizes ScriptTask before Task E run.