1660893 - Crash in [@ core::option::expect_none_failed | remote::startup::handler::new_remote_agent_handler]

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/e9fe8304-cb7d-4811-ba6d-3d66e0200824

Top 10 frames of crashing thread:

0 xul.dll RustMozCrash mozglue/static/rust/wrappers.cpp:16
1 xul.dll mozglue_static::panic_hook mozglue/static/rust/lib.rs:89
2 xul.dll core::ops::function::Fn::call<fn ../4fb7144ed159f94491249e86d5bbd033b5d60550/src/libcore/ops/function.rs:232
3 xul.dll std::panicking::rust_panic_with_hook ../4fb7144ed159f94491249e86d5bbd033b5d60550//src/libstd/panicking.rs:474
4 xul.dll std::panicking::begin_panic_handler ../4fb7144ed159f94491249e86d5bbd033b5d60550//src/libstd/panicking.rs:378
5 xul.dll core::panicking::panic_fmt ../4fb7144ed159f94491249e86d5bbd033b5d60550//src/libstd/panicking.rs:332
6 xul.dll core::option::expect_none_failed ../4fb7144ed159f94491249e86d5bbd033b5d60550//src/libcore/option.rs:1211
7 xul.dll remote::startup::handler::new_remote_agent_handler remote/startup/handler.rs:31
8 xul.dll GetRemoteAgentHandler remote/startup/RemoteAgentHandler.cpp:30
9 xul.dll mozilla::xpcom::CreateInstanceImpl xpcom/components/StaticComponents.cpp:10338

Filing this here because the remoting agent component doesn't support crash signatures. I spotted this during nightly crash triage but it doesn't seem like a new issue, the oldest crash report we have on file is for buildid 20200515215236.

Comment 1

[Andreas Tolfsen ❲:ato❳]

I’m no longer with Mozilla, but likely source is an underlying XPCOM exception.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks Andreas for letting me know! So that crash happens here:

https://searchfox.org/mozilla-central/rev/73a14f1b367948faa571ed2fe5d7eb29460787c1/remote/startup/handler.rs#30-34

#[no_mangle]
pub unsafe extern "C" fn new_remote_agent_handler(result: *mut *const nsICommandLineHandler) {
    let handler: RefPtr<RemoteAgentHandler> = RemoteAgentHandler::new().unwrap();
    RefPtr::new(handler.coerce::<nsICommandLineHandler>()).forget(&mut *result);
}

Not sure about which line of this code given that the crash report above only references the function header. But could this be the call to unwrap() and we fail to create a new instance of RemoteAgentHandler? James, what do you think?

Gabriele, what would be needed to get support for crash signatures?

Comment 3

[Julien Cristau [:jcristau] (back April 22)]

The crash reason is called `Result::unwrap()` on an `Err` value: Unavailable fwiw

Comment 4

[James Graham [:jgraham]]

So looking at the code I'm pretty sure the unwrap is failing. The fallible parts of the callee are basically xpcom::services::get_RemoteAgent() and xpcom::services::get_ObserverService() so ato is of course correct that the underlying failure is likely an XPCOM exception in one of those methods. I don't think the code currently allows us to tell more specifically what's going wrong, although we could try to propagate more information. In any case I'm not sure how to fix this kind of thing, we probably need to ask someone with more XPCOM experience.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, in that case moving the bug under XPCOM again. Nathan, do you have XPCOM knowledge, or at least know someone who could help with issues in xpcom::services::get_ObserverService()?

Comment 6

[Nathan Froyd [:froydnj]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #5)

Ok, in that case moving the bug under XPCOM again. Nathan, do you have XPCOM knowledge, or at least know someone who could help with issues in xpcom::services::get_ObserverService()?

get_ObserverService is a pretty trivial function:

https://searchfox.org/mozilla-central/source/__GENERATED__/__linux64__/xpcom/build/services.rs#50

Likewise with get_RemoteAgent:

https://searchfox.org/mozilla-central/source/__GENERATED__/__linux64__/xpcom/build/services.rs#203

And the generated C function(s) they call are likewise trivial:

https://searchfox.org/mozilla-central/source/__GENERATED__/__linux64__/xpcom/build/Services.cpp#145

So the question really is: why is the underlying service getter not working correctly? I don't really have a good answer for that; I think you'd need to modify the generated code to propagate a richer error up from the service creation to the Rust code.

Comment 7

[Andrew McCreight [:mccr8]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #2)

Gabriele, what would be needed to get support for crash signatures?

What do you mean by this? Do you mean making the crash reason publicly visible? We deliberately do not do that for Rust crashes because some Rust string handling code puts the contents of the string into the error, and there was a case where we were crashing in URL handling Rust code, so we were making URLs visible in crash reports, which is bad.

Comment 8

[Andrew McCreight [:mccr8]]

Never mind, I just read comment 0 that explains what that means.

Comment 9

[Nika Layzell [:nika] (ni? for response)]

It seems you're unwrapping the result of trying to fetch a service, and the service is failing. I'm guessing the failing service is the remote agent service. Fetching a JS-implemented service always has a chance to fail (e.g. due to an OOM), so you definitely want to properly handle and recover from any service getter failures.

The easiest fix is probably to remove the unwrap() call from new_remote_agent_handler (https://searchfox.org/mozilla-central/rev/d54712b9644b49cec6cc90a9e0c325fdfab04e7c/remote/startup/handler.rs#32), and instead initialize the outparam to null in the case where you fail to construct the service.

if let Some(handler) = RemoteAgentHandler::new() {
    RefPtr::new(handler.coerce::<nsICommandLineHandler>()).forget(&mut *result);
} else {
    *result = std::ptr::null();
}

Comment 10

[Gabriele Svelto [:gsvelto]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #2)

Gabriele, what would be needed to get support for crash signatures?

I feel super-silly because now that I'v double-checked it bugs in the Remote Protocol component seem to support it. It's just that you can't add a crash signature when creating a new bug, but after creating one it seems possible to add the crash signature.

Comment 11

[Julien Cristau [:jcristau] (back April 22)]

Is anyone working on this? 82 is about to merge to beta, and we're seeing a decent amount of crashes here on nightly.

Comment 12

[James Graham [:jgraham]]

I've taken Nika's patch and will get it through review.

Comment 13

[James Graham [:jgraham]]

Attachment: Bug 1660893 - Don't crash when the remote agent can't be initialized,

Set the command line handler to null when we can't start the service

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3803f3a513e4
Don't crash when the remote agent can't be initialized, r=remote-protocol-reviewers,whimboo

Comment 15

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/3803f3a513e4