Canvas rendering context can be destroyed after the canvas thread exits #14002

Closed
jrmuizel opened this Issue Oct 31, 2016 · 13 comments

Projects

None yet

7 participants

@jrmuizel

This happens on a simple canvas demo https://people-mozilla.org/~jmuizelaar/webrender/perf/canvas-perf.html

called Result::unwrap() on an Err value: Error { repr: Custom(Custom { kind: NotFound, error: StringError("Bogus destination port.") }) }

URL:

Servo Version:

Servo 0.0.1-b489ba0

Backtrace:


WARNING: <Constellation>: Finding mozbrowser ancestor for pipeline (1,5) after closure.
WARNING: <Constellation>: Closing pipeline PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(5) } twice.
WARNING: <Constellation>: creating replacement pipeline for about:failure
WARNING: <ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(5) }>: layout thread unreachable - leaking layout data
WARNING: <ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(5) }>: layout thread unreachable - leaking layout data
WARNING: <ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(5) }>: layout thread unreachable - leaking layout data
WARNING: <ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(5) }>: layout thread unreachable - leaking layout data
WARNING: <ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(5) }>: layout thread unreachable - leaking layout data
WARNING: <ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(5) }>: layout thread unreachable - leaking layout data
WARNING: <ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(5) }>: layout thread unreachable - leaking layout data
WARNING: <ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(5) }>: layout thread unreachable - leaking layout data
WARNING: <ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(5) }>: Panic hook called.
ERROR: called `Result::unwrap()` on an `Err` value: Error { repr: Custom(Custom { kind: NotFound, error: StringError("Bogus destination port.") }) }

stack backtrace:
   0:        0x10b4d571e - backtrace::backtrace::trace::h6ac07ced1b846a59
   1:        0x10b4d5a0c - backtrace::capture::Backtrace::new::h1d3339ed08c7f861
   2:        0x10a1c9d55 - constellation::constellation::log_entry::hc92ce9b47dde5c21
   3:        0x10a1c8c22 - _<constellation..constellation..FromCompositorLogger as log..Log>::log::h27ef5afa70002f0a
   4:        0x10b97d0b1 - log::__log::hf7539da7e3bbacdb
   5:        0x109fdc8b0 - servo::main::_{{closure}}::h70a6b1b48115735a
   6:        0x10b994153 - std::panicking::rust_panic_with_hook::hcd9d05f53fa0dafc
   7:        0x10b994024 - std::panicking::begin_panic::hf6c488cee66e7f17
   8:        0x10b993f42 - std::panicking::begin_panic_fmt::hb0a7126ee57cdd27
   9:        0x10b993ea7 - rust_begin_unwind
  10:        0x10b9c0c70 - core::panicking::panic_fmt::h9af671b78898cdba
  11:        0x10a5c2c65 - core::result::unwrap_failed::h3ea0c9548cd89371
  12:        0x10a5d13cf - drop::h33071f9ec9dcdc38
  13:        0x10b99517a - __rust_maybe_catch_panic
  14:        0x10a91bb0b - script::dom::bindings::codegen::Bindings::CanvasRenderingContext2DBinding::CanvasRenderingContext2DBinding::_finalize::hfb37a675f1388264
  15:        0x10ae3c280 - _ZN2js2gc5Arena8finalizeI8JSObjectEEmPNS_6FreeOpENS0_9AllocKindEm
  16:        0x10ae1657c - 2js2gc10ArenaLists30queueForegroundObjectsForSweepEPNS_6FreeOp
  17:        0x10ae1c930 - 2js2gc9GCRuntime22beginSweepingZoneGroupERNS_26AutoLockForExclusiveAccess
  18:        0x10ae1d47e - 2js2gc9GCRuntime15beginSweepPhaseEbRNS_26AutoLockForExclusiveAccess
  19:        0x10ae1f709 - 2js2gc9GCRuntime23incrementalCollectSliceERNS_11SliceBudgetEN2JS8gcreason6ReasonERNS_26AutoLockForExclusiveAccess
  20:        0x10ae1ffbb - 2js2gc9GCRuntime7gcCycleEbRNS_11SliceBudgetEN2JS8gcreason6Reason
  21:        0x10ae20503 - 2js2gc9GCRuntime7collectEbNS_11SliceBudgetEN2JS8gcreason6Reason
  22:        0x10ae207f8 - 2js2gc9GCRuntime2gcE18JSGCInvocationKindN2JS8gcreason6Reason
  23:        0x10af17919 - _ZN9JSRuntime14destroyRuntimeEv
  24:        0x10ade0136 - _ZN2js14DestroyContextEP9JSContext
  25:        0x10a3cfa5c - drop_contents::hb97c516d7ddca66a
  26:        0x10a4b7f42 - std::panicking::try::do_call::h78b74ac9f04f00a1
  27:        0x10b99517a - __rust_maybe_catch_panic
  28:        0x10a5f9886 - _<F as alloc..boxed..FnBox<A>>::call_box::h7e0283851864c35b
  29:        0x10b993084 - std::sys::thread::Thread::new::thread_start::h50b05608a499d2b2
  30:     0x7fff92dee898 - _pthread_body
  31:     0x7fff92dee729 - _pthread_start

This report was generated by the browser.html issue reporter.

@rzambre
Contributor
rzambre commented Nov 7, 2016 edited

This error occurs every time I quit Servo. This is also interfering with the -o out.png functionality. Tracked here: #14117

@jdm
Member
jdm commented Dec 13, 2016

This can be fixed by using let _ = self.ipc_renderer.send(...); instead of self.ipc_renderer.send(...).unwrap() in the drop method for CanvasRenderingContext2D.

Code: components/script/dom/canvasrenderingcontext2d.rs

I don't think we can write an automated test for this, so if it builds and the manual testcase doesn't panic it's good enough to submit.

@jdm jdm changed the title from called `Result::unwrap()` on an `Err` value: Error { repr: Custom(Custom { kind: NotFound, error: StringError("Bogus destination port.") }) } to Canvas rendering context can be destroyed after the canvas thread exits Dec 13, 2016
@jdm jdm added the E-easy label Dec 13, 2016
@highfive

Please make a comment here if you intend to work on this issue. Thank you!

@sam09
sam09 commented Dec 14, 2016

@highfive I would like to work on this.

@KiChjang
Member

@sam09 You got it!

(p.s. highfive is a bot, they do not understand commands)

@KiChjang KiChjang added the C-assigned label Dec 14, 2016
@sam09
sam09 commented Dec 14, 2016

@KiChjang Thanks a lot.

@prampey
Contributor
prampey commented Dec 25, 2016

@jdm Isn't it better if we legibly print out the error instead of masking it with let _ = self.ipc_renderer.send(...); ?

@jdm
Member
jdm commented Dec 25, 2016

It depends. I wouldn't complain about a patch that used warn! to report the error if it occurred.

@prampey
Contributor
prampey commented Dec 25, 2016

Oh okay. This seems cleaner to me. Anyways, the assignee seems to be inactive. Can I work on this?

@jdm
Member
jdm commented Dec 26, 2016

Please do!

@prampey
Contributor
prampey commented Dec 28, 2016

@jdm Since I'm only checking for an error, I've decided to use this:

if let Err(err) = self.ipc_renderer.send(CanvasMsg::Common(CanvasCommonMsg::Close)){
            println!("Error: {}",err)
        }
@jdm
Member
jdm commented Dec 28, 2016

@prampey Please use warn! instead of println!, since that integrates better with our error reporter. Additionally, an error message like Couldn't close canvas: {} will be more meaningful.

@prampey prampey referenced this issue Dec 28, 2016
Merged

Error handled canvas closing #14762

4 of 5 tasks complete
@prampey
Contributor
prampey commented Dec 28, 2016

@jdm Cool. Just sent in a PR.

@bors-servo bors-servo added a commit that referenced this issue Dec 28, 2016
@bors-servo bors-servo Auto merge of #14762 - prampey:error-handle, r=jdm
Error handled canvas closing

<!-- Please describe your changes on the following line: -->
Correctly handled error when Canvas doesn't close properly, with a descriptive warning.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #14002  (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because minor changes don't require tests.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14762)
<!-- Reviewable:end -->
c6ea1ec
@bors-servo bors-servo closed this in #14762 Dec 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment