Threaded Worklet crashes when reporting an exception |
|||||||
Issue description
If I have a syntax error in threaded worklet modules it leads to a crash in debug mode.
Here is the stacktrace where we hit a DCHECK():
[1:10:1102/162856.529863:FATAL:V8ScriptRunner.cpp(811)] Check failed: IsMainThread().
#0 0x0000074dd37d base::debug::StackTrace::StackTrace()
#1 0x0000074db90c base::debug::StackTrace::StackTrace()
#2 0x0000075527fa logging::LogMessage::~LogMessage()
#3 0x00000a67922c blink::V8ScriptRunner::ReportException()
#4 0x00000a65a9c8 blink::ScriptModule::ReportException()
#5 0x00000b3b66b9 blink::ModulatorImplBase::ExecuteModule()
#6 0x00000c3f8da1 blink::WorkletModuleTreeClient::NotifyModuleTreeLoadFinished()
#7 0x00000c03cef9 blink::ModuleTreeLinker::AdvanceState()
#8 0x00000c03dec7 blink::ModuleTreeLinker::Instantiate()
#9 0x00000c03dbf8 blink::ModuleTreeLinker::FinalizeFetchDescendantsForOneModuleScript()
#10 0x00000c03d245 blink::ModuleTreeLinker::FetchDescendants()
#11 0x00000c03db9f blink::ModuleTreeLinker::NotifyModuleLoadFinished()
Here is a simple repro:
content_shell --enable-experimental-web-platform-features --allow-file-access-from-files --disable-web-security "data:text/html,<script>window.animationWorklet.addModule(URL.createObjectURL(new Blob([\"function();\"], {type: 'text/javascript'})))</script>"
,
Nov 6 2017
,
Nov 6 2017
V8ScriptRunner::ReportException() is used for parse errors (and instantiation errors) to "report the error", but V8ScriptRunner::ReportException() currently supports main thread only, and thus the DCHECK() is failing. There is a TODO comment there: // TODO(adamk): Handle calls on worker threads. Probably we should fix this TODO? For evaluation errors, v8::TryCatch with SetVerbose(true) is used to "report the error", which seems to support worker threads, and thus Chromium doesn't crash. This case is tested by a wpt test: external/wpt/worklets/animation-worklet-import.html.
,
Nov 6 2017
The right fix for this should be to expose MessageHandlerInWorker (which is currently file-scoped to V8Initializer.cpp) as a static method on V8Initializer. A quick glance through the code suggests that it might just work to call it from V8ScriptRunner::ReportException (if in the !IsMainThread() case).
,
Nov 7 2017
nhiroki: Would you handle this?
,
Nov 7 2017
Sure. I'll take a look...
,
Nov 7 2017
CL is under review: https://chromium-review.googlesource.com/c/chromium/src/+/756096
,
Nov 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e5bfcd45e2bdfaf18b188c8b517cf8143fb8575e commit e5bfcd45e2bdfaf18b188c8b517cf8143fb8575e Author: Hiroki Nakagawa <nhiroki@chromium.org> Date: Wed Nov 08 03:33:05 2017 Worklet: Implement exception report path for ThreadedWorklet This CL implements exception report path for ThreadedWorklet. Specifically, this exposes MessageHandlerInWorker() that is a file-scoped static function to V8Initializer, and makes V8ScriptRunner::ReportException() call it when it's on a worker thread. For confirming this fix, this CL adds WPT tests for a syntax error and an invalid module specifier on worklets, and fixes error handling in WorkletModuleTreeClient::NotifyModuleTreeLoadFinished(). Bug: 781300 Change-Id: Id9be208ecbc594b4e33cb07279f5b987107289e5 Reviewed-on: https://chromium-review.googlesource.com/756096 Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Adam Klein <adamk@chromium.org> Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#514733} [add] https://crrev.com/e5bfcd45e2bdfaf18b188c8b517cf8143fb8575e/third_party/WebKit/LayoutTests/external/wpt/worklets/resources/import-syntax-error-worklet-script.js [modify] https://crrev.com/e5bfcd45e2bdfaf18b188c8b517cf8143fb8575e/third_party/WebKit/LayoutTests/external/wpt/worklets/resources/import-tests.js [add] https://crrev.com/e5bfcd45e2bdfaf18b188c8b517cf8143fb8575e/third_party/WebKit/LayoutTests/external/wpt/worklets/resources/syntax-error-worklet-script.js [modify] https://crrev.com/e5bfcd45e2bdfaf18b188c8b517cf8143fb8575e/third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp [modify] https://crrev.com/e5bfcd45e2bdfaf18b188c8b517cf8143fb8575e/third_party/WebKit/Source/bindings/core/v8/V8Initializer.h [modify] https://crrev.com/e5bfcd45e2bdfaf18b188c8b517cf8143fb8575e/third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp [modify] https://crrev.com/e5bfcd45e2bdfaf18b188c8b517cf8143fb8575e/third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.cpp
,
Nov 8 2017
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by majidvp@chromium.org
, Nov 3 2017