Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: crash on WebWorkerObserver script execution #37081

Conversation

trop[bot]
Copy link
Contributor

@trop trop bot commented Jan 31, 2023

Backport of #37050

See that PR for details.

Notes: Fixed a potential crash in some types of Worklets.

@trop trop bot requested a review from codebytere January 31, 2023 11:29
@trop trop bot added 23-x-y backport This is a backport PR semver/patch backwards-compatible bug fixes labels Jan 31, 2023
@codebytere codebytere closed this Jan 31, 2023
@codebytere codebytere deleted the trop/23-x-y-bp-fix-crash-on-webworkerobserver-script-execution-1675164584408 branch January 31, 2023 12:56
@kamalbennani
Copy link

Hi @codebytere
Is there a reason why this PR has been closed? do you have any ETA to communicate when this could land on the electron@23 tag.
Thanks.

@codebytere codebytere restored the trop/23-x-y-bp-fix-crash-on-webworkerobserver-script-execution-1675164584408 branch February 7, 2023 09:17
@codebytere codebytere reopened this Feb 7, 2023
@rmnbrd
Copy link

rmnbrd commented Feb 8, 2023

Hello @codebytere !
Thanks for reopening this backport PR 🙏

I noticed the tests seem to fail. Could we be of any help here?

@codebytere
Copy link
Member

This seems to actually be a different failure than the original one fixed in this PR:

#
# Fatal error in ../../v8/src/api/api-inl.h, line 182
# Debug check failed: microtask_queue->GetMicrotasksScopeDepth() || !microtask_queue->DebugMicrotasksScopeDepthIsZero().
#
#
#
#FailureMessage Object: 0x177465d78
0   Electron Framework                  0x00000001184a1e28 base::debug::CollectStackTrace(void**, unsigned long) + 28
1   Electron Framework                  0x00000001183dd270 base::debug::StackTrace::StackTrace() + 24
2   Electron Framework                  0x0000000119ca29b8 gin::(anonymous namespace)::PrintStackTrace() + 40
3   Electron Framework                  0x0000000119a9e7f0 V8_Fatal(char const*, int, char const*, ...) + 268
4   Electron Framework                  0x0000000119a9e494 std::Cr::enable_if<!std::is_function<std::Cr::remove_pointer<unsigned char>::type>::value && !std::is_enum<unsigned char>::value && has_output_operator<unsigned char, v8::base::CheckMessageStream>::value, std::Cr::basic_string<char, std::Cr::char_traits<char>, std::Cr::allocator<char>>>::type v8::base::PrintCheckOperand<unsigned char>(unsigned char) + 0
5   Electron Framework                  0x0000000115ad8d24 v8::CallDepthScope<true>::~CallDepthScope() + 1156
6   Electron Framework                  0x0000000115a8bc68 v8::Object::Set(v8::Local<v8::Context>, v8::Local<v8::Value>, v8::Local<v8::Value>) + 296
7   Electron Framework                  0x000000011cc79ca8 node::InitializePrimordials(v8::Local<v8::Context>) + 156
8   Electron Framework                  0x000000011cc79bc4 node::GetPerContextExports(v8::Local<v8::Context>) + 208
9   Electron Framework                  0x000000011cc79c90 node::InitializePrimordials(v8::Local<v8::Context>) + 132
10  Electron Framework                  0x000000011cc7a2b0 node::InitializeMainContextForSnapshot(v8::Local<v8::Context>) + 112
11  Electron Framework                  0x000000011cc79e18 node::InitializeContext(v8::Local<v8::Context>) + 20
12  Electron Framework                  0x000000011489b44c electron::WebWorkerObserver::WorkerScriptReadyForEvaluation(v8::Local<v8::Context>) + 104
13  Electron Framework                  0x000000011a01ebe0 blink::WorkerOrWorkletScriptController::PrepareForEvaluation() + 412
14  Electron Framework                  0x000000011a01e214 blink::WorkerOrWorkletScriptController::Initialize(blink::KURL const&) + 1056
15  Electron Framework                  0x000000011b095944 blink::WorkerThread::InitializeOnWorkerThread(std::Cr::unique_ptr<blink::GlobalScopeCreationParams, std::Cr::default_delete<blink::GlobalScopeCreationParams>>, absl::optional<blink::WorkerBackingThreadStartupData> const&, std::Cr::unique_ptr<blink::WorkerDevToolsParams, std::Cr::default_delete<blink::WorkerDevToolsParams>>) + 748
16  Electron Framework                  0x000000011b0999ac void base::internal::FunctorTraits<void (blink::WorkerThread::*)(std::Cr::unique_ptr<blink::GlobalScopeCreationParams, std::Cr::default_delete<blink::GlobalScopeCreationParams>>, absl::optional<blink::WorkerBackingThreadStartupData> const&, std::Cr::unique_ptr<blink::WorkerDevToolsParams, std::Cr::default_delete<blink::WorkerDevToolsParams>>), void>::Invoke<void (blink::WorkerThread::*)(std::Cr::unique_ptr<blink::GlobalScopeCreationParams, std::Cr::default_delete<blink::GlobalScopeCreationParams>>, absl::optional<blink::WorkerBackingThreadStartupData> const&, std::Cr::unique_ptr<blink::WorkerDevToolsParams, std::Cr::default_delete<blink::WorkerDevToolsParams>>), blink::WorkerThread*, std::Cr::unique_ptr<blink::GlobalScopeCreationParams, std::Cr::default_delete<blink::GlobalScopeCreationParams>>, absl::optional<blink::WorkerBackingThreadStartupData>, std::Cr::unique_ptr<blink::WorkerDevToolsParams, std::Cr::default_delete<blink::WorkerDevToolsParams>>>(void (blink::WorkerThread::*)(std::Cr::unique_ptr<blink::GlobalScopeCreationParams, std::Cr::default_delete<blink::GlobalScopeCreationParams>>, absl::optional<blink::WorkerBackingThreadStartupData> const&, std::Cr::unique_ptr<blink::WorkerDevToolsParams, std::Cr::default_delete<blink::WorkerDevToolsParams>>), blink::WorkerThread*&&, std::Cr::unique_ptr<blink::GlobalScopeCreationParams, std::Cr::default_delete<blink::GlobalScopeCreationParams>>&&, absl::optional<blink::WorkerBackingThreadStartupData>&&, std::Cr::unique_ptr<blink::WorkerDevToolsParams, std::Cr::default_delete<blink::WorkerDevToolsParams>>&&) + 76
17  Electron Framework                  0x000000011b099954 base::internal::Invoker<base::internal::BindState<void (blink::WorkerThread::*)(std::Cr::unique_ptr<blink::GlobalScopeCreationParams, std::Cr::default_delete<blink::GlobalScopeCreationParams>>, absl::optional<blink::WorkerBackingThreadStartupData> const&, std::Cr::unique_ptr<blink::WorkerDevToolsParams, std::Cr::default_delete<blink::WorkerDevToolsParams>>), WTF::CrossThreadUnretainedWrapper<blink::WorkerThread>, std::Cr::unique_ptr<blink::GlobalScopeCreationParams, std::Cr::default_delete<blink::GlobalScopeCreationParams>>, absl::optional<blink::WorkerBackingThreadStartupData>, std::Cr::unique_ptr<blink::WorkerDevToolsParams, std::Cr::default_delete<blink::WorkerDevToolsParams>>>, void ()>::RunOnce(base::internal::BindStateBase*) + 48
18  Electron Framework                  0x00000001184417d0 base::TaskAnnotator::RunTaskImpl(base::PendingTask&) + 284
19  Electron Framework                  0x0000000118464750 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::LazyNow*) + 1232
20  Electron Framework                  0x0000000118463d9c base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() + 108
21  Electron Framework                  0x00000001183faa68 base::MessagePumpDefault::Run(base::MessagePump::Delegate*) + 120
22  Electron Framework                  0x0000000118465494 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) + 432
23  Electron Framework                  0x0000000118420550 base::RunLoop::Run(base::Location const&) + 448
24  Electron Framework                  0x00000001170e125c blink::scheduler::NonMainThreadImpl::SimpleThreadImpl::Run() + 260
25  Electron Framework                  0x00000001184ae408 base::(anonymous namespace)::ThreadFunc(void*) + 132
26  libsystem_pthread.dylib             0x00000001b201d06c _pthread_start + 148
27  libsystem_pthread.dylib             0x00000001b2017e2c thread_start + 8

it also happens without these changes, leading me to believe this is also a function of different Node.js versions between 23 and main.

@rmnbrd
Copy link

rmnbrd commented Feb 8, 2023

Oh ok, makes sense now!

Out of curiosity, would there be a way for us to test these changes in the meantime?
And if so, what would be the process to follow?

@codebytere
Copy link
Member

@rmnbrd i found it - it's #36870. I can likely backport that PR first, and then once that's in, this should be unblocked. Testing it now to validate.

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
@codebytere codebytere force-pushed the trop/23-x-y-bp-fix-crash-on-webworkerobserver-script-execution-1675164584408 branch from 6ca2ec9 to fb39bed Compare February 9, 2023 09:32
@kamalbennani
Copy link

The checks SUCCEEDED 🎉
Thank you @codebytere for spending time on this. really appreciated

@codebytere codebytere merged commit f99c175 into 23-x-y Feb 9, 2023
@codebytere codebytere deleted the trop/23-x-y-bp-fix-crash-on-webworkerobserver-script-execution-1675164584408 branch February 9, 2023 15:19
@release-clerk
Copy link

release-clerk bot commented Feb 9, 2023

Release Notes Persisted

Fixed a potential crash in some types of Worklets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
23-x-y backport This is a backport PR semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants