Closed Bug 1542541 Opened 6 years ago Closed 6 years ago

FactoryOp::ActorDestroy needs to do cleanup if the actor was waiting for the PermissionRetry message

Categories

(Core :: Storage: IndexedDB, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox67 --- fixed
firefox68 --- fixed

People

(Reporter: janv, Assigned: janv)

References

Details

Crash Data

Attachments

(1 file)

Ok, this is not a sec issue, because gFactoryOps holds strong references to FactoryOp objects, but if we don't do the cleanup, the op is not removed from gFactoryOps array and that leads to a shutdown hang.

This all can happen when parent sends PermissionChallenge to the child and the child dies instead of sending PermissionRetry back to the parent.

I guess the permission stuff is still used by extensions.

I can definitely simulate this issue with dom/indexedDB/test/browser_permissionsPromptAllow.js test.

We should fix it.

Assignee: nobody → jvarga
Blocks: 1541370
Status: NEW → ASSIGNED
Summary: FactoryOp::ActoryDestroy needs to do cleanup if the actor was waiting for the PermissionRetry message → FactoryOp::ActorDestroy needs to do cleanup if the actor was waiting for the PermissionRetry message
Attachment #9056357 - Attachment description: Bug 1542541 - FactoryOp::ActoryDestroy needs to do cleanup if the actor was waiting for the PermissionRetry message; r=asuth → Bug 1542541 - FactoryOp::ActorDestroy needs to do cleanup if the actor was waiting for the PermissionRetry message; r=asuth
Pushed by jvarga@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/42fe05233d68 FactoryOp::ActorDestroy needs to do cleanup if the actor was waiting for the PermissionRetry message; r=asuth
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Crash Signature: [@ mozilla::dom::indexedDB::`anonymous namespace'::QuotaClient::ShutdownTimedOut] [@ mozilla::dom::indexedDB::(anonymous namespace)::QuotaClient::ShutdownTimedOut]

There's a chance that this has had an impact on IDB shutdown hangs. We'll see next week.

Comment on attachment 9056357 [details]
Bug 1542541 - FactoryOp::ActorDestroy needs to do cleanup if the actor was waiting for the PermissionRetry message; r=asuth

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: This has been an issue for long time
  • User impact if declined: The number of shutdown hangs stays high. This patch should reduce them.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): A simple patch, verified on Nightly, covered by tests.
  • String changes made/needed: None
Attachment #9056357 - Flags: approval-mozilla-beta?

Comment on attachment 9056357 [details]
Bug 1542541 - FactoryOp::ActorDestroy needs to do cleanup if the actor was waiting for the PermissionRetry message; r=asuth

Low risk patch with tests that may reduce our shutdown hangs crashes, uplift approved for 67 beta 12, thanks.

Attachment #9056357 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Crash Signature: [@ mozilla::dom::indexedDB::`anonymous namespace'::QuotaClient::ShutdownTimedOut] [@ mozilla::dom::indexedDB::(anonymous namespace)::QuotaClient::ShutdownTimedOut] → [@ mozilla::dom::indexedDB::`anonymous namespace'::QuotaClient::ShutdownTimedOut] [@ mozilla::dom::indexedDB::(anonymous namespace)::QuotaClient::ShutdownTimedOut] [@ shutdownhang | mozilla::SpinEventLoopUntil<T> | mozilla::dom::quota::QuotaManager::Obs…
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: