Closed Bug 1769485 Opened 2 years ago Closed 2 years ago

Assert that we do not skip ShutdownPhase::XPCOMShutdown due to a missing service manager

Categories

(Core :: XPCOM, task)

task

Tracking

()

RESOLVED FIXED
103 Branch
Tracking Status
firefox103 --- fixed

People

(Reporter: jstutte, Assigned: archdevx7d6)

References

Details

(Keywords: good-first-bug)

Attachments

(2 files, 2 obsolete files)

We advance to ShutdownPhase::XPCOMShutdown only if we find the service manager.

The current code looks as if we would treat the error, but we actually do not expect this to be ever missing at this stage of shutdown and just skipping it would be bad. Thus we want to add a MOZ_DIAGNOSTIC_ASSERT here to see if our assumptions hold.

I would like to work on this bug.

Attached patch XPCOMInit.cppSplinter Review

I tried to fix the bug. I am new to Mozilla community so I would like to ask for some guidance.

Hi! Great that you want to help!
However, you should submit a patch, not just a modified source file. Please use moz-phab to submit the patch, such that we can do the review inside phabricator which eases life for all of us.
Please be sure to have read Firefox Contributors’ Quick Reference and possibly Sending your code for review (also known as “sending patches”). It can feel kind of overwhelming at the beginning, but it definitely pays off once you are through it.
Thanks for your support!

Flags: needinfo?(archdevx7d6)

I'm sorry, you're right. I will send a patch file in a moment

Flags: needinfo?(archdevx7d6)
Assignee: nobody → archdevx7d6
Status: NEW → ASSIGNED
Attachment #9278972 - Attachment is obsolete: true
Attachment #9278757 - Attachment is obsolete: true

Hi, sorry for the hassle! I just realized that you seem to use git, not mercurial. Honestly I do not know how to configure that to work correctly with moz-phab - I am sure there is a way, though. I'll ask around.

Flags: needinfo?(archdevx7d6)

that will set the metadata and then it's necessary to amend the commit like git commit --amend --reset-author

Pushed by jstutte@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1b8326b00bed
Replace if statement checking if service manager is present with MOZ_DIAGNOSTIC_ASSERT. r=xpcom-reviewers,jstutte
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 103 Branch

Congrats!

Flags: needinfo?(archdevx7d6)
Regressions: 1772523
You need to log in before you can comment on or make changes to this bug.