Closed Bug 1766572 Opened 11 months ago Closed 11 months ago

Audit AdvanceShutdownPhase to be more reliable in late shutdown

Categories

(Core :: XPCOM, defect)

Product:
Core
Component:
XPCOM
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
102 Branch
Tracking Flags:
Tracking Status
firefox102 --- fixed

People

(Reporter: jstutte, Assigned: jstutte)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files, 2 obsolete files)

Bug 1766572: Part 1 - Remove obsolete ShutdownPhase::XPCOMShutdownLoaders. r?#xpcom-reviewers
48 bytes, text/x-phabricator-request
Details | Review
Bug 1766572: Part 2 - Synchronize KillClearOnShutdown phases with AdvanceShutdownPhase. r?dthayer,#xpcom-reviewers!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1766572: Part 3 - Make nsTerminator aware of late shutdown phases. r?#xpcom-reviewers
48 bytes, text/x-phabricator-request
Details | Review

From a first look it seems we never advance sCurrentShutdownPhase for the phases later than XPCOMShutdownThreads. But we already introduced checks that rely on this in:

These are effectively no-ops.

See Also: → 1762604, 1764251
See Also: → 1697745

We could sync the shutdown state between ClearOnShutdown and AppShutdown as a first step towards bug 1697745.

Assignee: nobody → jstutte
Status: NEW → ASSIGNED

AppShutdown::AdvanceShutdownPhase tried to preserved the original order we had historically in XPCOMShutdown. This resulted in having called KillClearOnShutdown before we tried fast shutdown and/or notified our observers.
It seems desirable to remove the objects associated via ClearOnShutdown(aPhase) as last operation here, such that:

  • fast shutdown can just override their (then useless) deletion
  • otherwise notified observers can rely on them to still exist when executed
Blocks: 1697745
See Also: 1697745 →
Blocks: 1762840
Attachment #9274456 - Attachment description: Bug 1766572: Part 1 - Remove obsolete ShutdownPhase::XPCOMShutdownLoaders. r?#xpcom-reviewers → Bug 1766572: Part 1 - Rename ShutdownPhase::XPCOMShutdownLoaders to XPCOMShutdownMainThread. r?#xpcom-reviewers
Attachment #9274457 - Attachment description: Bug 1766572: Part 2 - Run KillClearOnShutdown only after having notified observers in AdvanceShutdownPhase. r?#xpcom-reviewers → Bug 1766572: Part 2 - Run KillClearOnShutdown only after having notified observers in AdvanceShutdownPhase. r?#xpcom-reviewers,dthayer!
Attachment #9274458 - Attachment description: Bug 1766572: Part 3 - Call KillClearOnShutdown only from AdvanceShutdownPhase(WithoutNotify). r?#xpcom-reviewers → Bug 1766572: Part 3 - Synchronize KillClearOnShutdown phases with AdvanceShutdownPhase(WithoutNotify). r?#xpcom-reviewers
Attachment #9274611 - Attachment description: Bug 1766572: Part 4 - Adjust some comments as AdvanceShutdownPhaseWithoutNotify will stay with us. r?#xpcom-reviewers → Bug 1766572: Part 5 - Adjust some comments as AdvanceShutdownPhaseWithoutNotify will stay with us. r?#xpcom-reviewers
Attachment #9274456 - Attachment description: Bug 1766572: Part 1 - Rename ShutdownPhase::XPCOMShutdownLoaders to XPCOMShutdownMainThread. r?#xpcom-reviewers → WIP: Bug 1766572: Part 1 - Remove obsolete ShutdownPhase::XPCOMShutdownLoaders.
Attachment #9274457 - Attachment description: Bug 1766572: Part 2 - Run KillClearOnShutdown only after having notified observers in AdvanceShutdownPhase. r?#xpcom-reviewers,dthayer! → WIP: Bug 1766572: Part 2 - Synchronize KillClearOnShutdown phases with AdvanceShutdownPhase(WithoutNotify).
Attachment #9274973 - Attachment description: Bug 1766572: Part 4 - Make nsTerminator aware of late shutdown phases. r?#xpcom-reviewers → WIP: Bug 1766572: Part 3 - Make nsTerminator aware of late shutdown phases.
Attachment #9274611 - Attachment description: Bug 1766572: Part 5 - Adjust some comments as AdvanceShutdownPhaseWithoutNotify will stay with us. r?#xpcom-reviewers → WIP: Bug 1766572: Part 4 - Adjust some comments as AdvanceShutdownPhaseWithoutNotify will stay with us.
Attachment #9274458 - Attachment is obsolete: true
Attachment #9274611 - Attachment is obsolete: true
Attachment #9274456 - Attachment description: WIP: Bug 1766572: Part 1 - Remove obsolete ShutdownPhase::XPCOMShutdownLoaders. → Bug 1766572: Part 1 - Remove obsolete ShutdownPhase::XPCOMShutdownLoaders. r?#xpcom-reviewers
Attachment #9274457 - Attachment description: WIP: Bug 1766572: Part 2 - Synchronize KillClearOnShutdown phases with AdvanceShutdownPhase(WithoutNotify). → Bug 1766572: Part 2 - Synchronize KillClearOnShutdown phases with AdvanceShutdownPhase(WithoutNotify). r?dthayer!,#xpcom-reviewers!
Attachment #9274973 - Attachment description: WIP: Bug 1766572: Part 3 - Make nsTerminator aware of late shutdown phases. → Bug 1766572: Part 3 - Make nsTerminator aware of late shutdown phases. r?#xpcom-reviewers
See Also: → 1768581
Attachment #9274457 - Attachment description: Bug 1766572: Part 2 - Synchronize KillClearOnShutdown phases with AdvanceShutdownPhase(WithoutNotify). r?dthayer!,#xpcom-reviewers! → Bug 1766572: Part 2 - Synchronize KillClearOnShutdown phases with AdvanceShutdownPhase. r?dthayer,#xpcom-reviewers!
See Also: → 1768795
Blocks: 1709019
Pushed by jstutte@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/47a749623030
Part 1 - Remove obsolete ShutdownPhase::XPCOMShutdownLoaders. r=xpcom-reviewers,nika
https://hg.mozilla.org/integration/autoland/rev/77c9472e4932
Part 2 - Synchronize KillClearOnShutdown phases with AdvanceShutdownPhase. r=xpcom-reviewers,dthayer,nika
https://hg.mozilla.org/integration/autoland/rev/3031b4d8ca8b
Part 3 - Make nsTerminator aware of late shutdown phases. r=xpcom-reviewers,nika

https://hg.mozilla.org/mozilla-central/rev/47a749623030
https://hg.mozilla.org/mozilla-central/rev/77c9472e4932
https://hg.mozilla.org/mozilla-central/rev/3031b4d8ca8b

Status: ASSIGNED → RESOLVED
Closed: 11 months ago
status-firefox102: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch
You need to log in before you can comment on or make changes to this bug.