(In reply to Andrew McCreight [:mccr8] from comment #1) > It kind of looks like a regression in 129. This isn't enabled on all branches, but looking back at the last 6 months, I only see a single occurrence of this stack (from January: bp-45a0897a-8cbd-43f8-9205-daccb0240104) until the 20240613093628, and then there have been around a half dozen of these, which is a decent chunk of all crashes with this signature in the last 6 months. Note that the specific signatures here relate to the debugger interrupt mechanism introduced in bug 1821250 in Firefox 128. We do not expect this signature to have been possible before that point. Although some of the stacks are truncated, they generally show that we are interrupting top-level script evaluation which means we do have a nested event loop on the stack... which ties into: As discussed at the team meeting today: - Eden indicated he suspects [mozilla::dom::WorkerPrivate::GetCurrentEventLoopGlobal](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/workers/WorkerPrivate.h#498) is likely involved, which makes sense since it only returns non-null values when there is a syncloop. We [check that in WorkerThreadRunnable::Run](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/workers/WorkerRunnable.cpp#402-403) as part of changes in Fx127 in bug 1769913. - :smaug indicated it's weird how [DebuggerImmediateRunnable::WorkerRun manually invokes JS_CallFunctionValue (with the global)](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/workers/WorkerPrivate.cpp#671) after [pulling the function out of the dom::Function mHandler](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/workers/WorkerPrivate.cpp#665-666) where the CallbackObject straight up tracks the [mozilla::dom::CallbackObject::mCallbackGlobal](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/bindings/CallbackObject.h#320-324) and if we just had the CallbackObject/dom::Function do the call for us, we should be fine. - It makes sense that there probably was some weird evolutionary story that got us here, but our current calling is not a best practice.
Bug 1904059 Comment 4 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
(In reply to Andrew McCreight [:mccr8] from comment #1) > It kind of looks like a regression in 129. This isn't enabled on all branches, but looking back at the last 6 months, I only see a single occurrence of this stack (from January: bp-45a0897a-8cbd-43f8-9205-daccb0240104) until the 20240613093628, and then there have been around a half dozen of these, which is a decent chunk of all crashes with this signature in the last 6 months. Note that the specific signatures here relate to the debugger interrupt mechanism introduced in bug 1821250 in Firefox 128. We do not expect this exact signature to have been possible before that point, but the logic resulting in the mismatch for the general signature could have happened if like a sync XHR was on the stack. Although some of the stacks are truncated, they generally show that we are interrupting top-level script evaluation which means we do have a nested event loop on the stack... which ties into: As discussed at the team meeting today: - Eden indicated he suspects [mozilla::dom::WorkerPrivate::GetCurrentEventLoopGlobal](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/workers/WorkerPrivate.h#498) is likely involved, which makes sense since it only returns non-null values when there is a syncloop. We [check that in WorkerThreadRunnable::Run](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/workers/WorkerRunnable.cpp#402-403) as part of changes in Fx127 in bug 1769913. - :smaug indicated it's weird how [DebuggerImmediateRunnable::WorkerRun manually invokes JS_CallFunctionValue (with the global)](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/workers/WorkerPrivate.cpp#671) after [pulling the function out of the dom::Function mHandler](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/workers/WorkerPrivate.cpp#665-666) where the CallbackObject straight up tracks the [mozilla::dom::CallbackObject::mCallbackGlobal](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/bindings/CallbackObject.h#320-324) and if we just had the CallbackObject/dom::Function do the call for us, we should be fine. - It makes sense that there probably was some weird evolutionary story that got us here, but our current calling is not a best practice.
(In reply to Andrew McCreight [:mccr8] from comment #1) > It kind of looks like a regression in 129. This isn't enabled on all branches, but looking back at the last 6 months, I only see a single occurrence of this stack (from January: bp-45a0897a-8cbd-43f8-9205-daccb0240104) until the 20240613093628, and then there have been around a half dozen of these, which is a decent chunk of all crashes with this signature in the last 6 months. Note that the specific signatures here relate to the debugger interrupt mechanism introduced in bug 1821250 in Firefox 128. We do not expect this exact signature to have been possible before that point, but the logic resulting in the mismatch for the general signature could have happened if like a sync XHR was on the stack. Although some of the stacks are truncated, they generally show that we are interrupting top-level script evaluation which means we do have a nested event loop on the stack... which ties into: As discussed at the team meeting today: - Eden indicated he suspects [mozilla::dom::WorkerPrivate::GetCurrentEventLoopGlobal](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/workers/WorkerPrivate.h#498) is likely involved, which makes sense since it only returns non-null values when there is a syncloop. We [check that in WorkerThreadRunnable::Run](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/workers/WorkerRunnable.cpp#402-403) as part of changes in Fx127 in bug 1769913 to use it as the basis for the global we choose to use. - :smaug indicated it's weird how [DebuggerImmediateRunnable::WorkerRun manually invokes JS_CallFunctionValue (with the global)](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/workers/WorkerPrivate.cpp#671) after [pulling the function out of the dom::Function mHandler](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/workers/WorkerPrivate.cpp#665-666) where the CallbackObject straight up tracks the [mozilla::dom::CallbackObject::mCallbackGlobal](https://searchfox.org/mozilla-central/rev/8fffdc727aa507ee4955042ec2d6f71d23c9c2de/dom/bindings/CallbackObject.h#320-324) and if we just had the CallbackObject/dom::Function do the call for us, we should be fine. - It makes sense that there probably was some weird evolutionary story that got us here, but our current calling is not a best practice.