Bug 1542181 Comment 16 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 Tom Tung [:tt, :ttung] from comment #13)
> The summary of this bug says "profile-change-teardown" (also shows on the crash reports). And, it seems that this topic is fired before the topic for shutting down QuotaManager? [3] If it's, maybe we have to shutdown quotamanager eariler.

QuotaManager needs to remain operational through its current "profile-before-change-qm" phase[1].  However, I think it's reasonable for us to require that any callers that want to use QuotaManager storage beyond the "quit-application-granted" phase (https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Observer_Notifications#Application_shutdown) explicitly provide the nsIAsyncShutdownClient that represents the phase that is being blocked and the nsIAsyncShutdownBlocker that has been added as a blocker to the client.

Any DirectoryLock/IDB transaction/etc. that doesn't have an associated client/blocker will be terminated when the phase starts with an error-code indicating shutdown.  Any request to open a new directorylock/etc. whose phase has already passed will immediately fail with an error-code indicating shutdown.  Some level of being able to dynamically update the associated phases may potentially be necessary.  For example, the IndexedDB commit() method that was recently added to the spec (https://w3c.github.io/IndexedDB/#dom-idbtransaction-commit) is intended to be a durable commit, and it might make sense to allow the commit some extra leeway to commit as long as the commit is likely to conclude in a timely fashion.  (That is, if the amount of data is reasonable and we've already opened the database, etc.)

So in the case of extension data migration, given that it already bails if `nsIAppStartup.shuttingDown` is true and that flag gets set just prior to triggering "quit-application-granted", the extension code wouldn't need to change at all because our new default behavior would handle this.  It's likely that this is also the right decision for most other consumers as well, especially those that haven't thought about shutdown in much depth.

Note that the proposed choice of quit-application-granted may be a little bit too aggressive in the case of content principals.  We will need to audit shutdown[2] to make sure that all of the window globals in question will have been reliably terminated, with DOMEventTargetHelper::DisconnectFromOwner() having been invoked.  Our explicit goal would be that the global needs to not be alive to process any error message that result from the automatic termination of pending QuotaManager client requests.  It's possible that this might mean working with other teams to move up when window globals are torn down, etc.

1: In particular, QuotaManager exists as an important system primitive and if we want to see it widely and consistently used, it needs to be usable until late in the shutdown process.  That said, as the rest of the above text suggests, we do want to encourage consumers of QuotaManager to shutdown earlier.
2: My shutdown notes are at https://github.com/asutherland/asuth-gecko-notes/blob/master/ipc/SHUTDOWN.md but haven't been updated for a while at this moment and were never 100% thorough.
(In reply to Tom Tung [:tt, :ttung] from comment #13)
> The summary of this bug says "profile-change-teardown" (also shows on the crash reports). And, it seems that this topic is fired before the topic for shutting down QuotaManager? [3] If it's, maybe we have to shutdown quotamanager eariler.

QuotaManager needs to remain operational through its current "profile-before-change-qm" phase[1].  However, I think it's reasonable for us to require that any callers that want to use QuotaManager storage beyond the "quit-application-granted" phase (https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Observer_Notifications#Application_shutdown) explicitly provide the nsIAsyncShutdownClient that represents the phase that is being blocked and the nsIAsyncShutdownBlocker that has been added as a blocker to the client.

Any DirectoryLock/IDB transaction/etc. that doesn't have an associated client/blocker will be terminated when the "quit-application-granted" phase starts with an error-code indicating shutdown.  Any lock/transaction that does have a client/blocker will be terminated when that phase starts.  Any request to open a new directorylock/etc. whose phase has already started will immediately fail with an error-code indicating shutdown.  Some level of being able to dynamically update the associated phases may potentially be necessary.  For example, the IndexedDB commit() method that was recently added to the spec (https://w3c.github.io/IndexedDB/#dom-idbtransaction-commit) is intended to be a durable commit, and it might make sense to allow the commit some extra leeway to commit as long as the commit is likely to conclude in a timely fashion.  (That is, if the amount of data is reasonable and we've already opened the database, etc.)

So in the case of extension data migration, given that it already bails if `nsIAppStartup.shuttingDown` is true and that flag gets set just prior to triggering "quit-application-granted", the extension code wouldn't need to change at all because our new default behavior would handle this.  It's likely that this is also the right decision for most other consumers as well, especially those that haven't thought about shutdown in much depth.

Note that the proposed choice of quit-application-granted may be a little bit too aggressive in the case of content principals.  We will need to audit shutdown[2] to make sure that all of the window globals in question will have been reliably terminated, with DOMEventTargetHelper::DisconnectFromOwner() having been invoked.  Our explicit goal would be that the global needs to not be alive to process any error message that result from the automatic termination of pending QuotaManager client requests.  It's possible that this might mean working with other teams to move up when window globals are torn down, etc.

1: In particular, QuotaManager exists as an important system primitive and if we want to see it widely and consistently used, it needs to be usable until late in the shutdown process.  That said, as the rest of the above text suggests, we do want to encourage consumers of QuotaManager to shutdown earlier.
2: My shutdown notes are at https://github.com/asutherland/asuth-gecko-notes/blob/master/ipc/SHUTDOWN.md but haven't been updated for a while at this moment and were never 100% thorough.

Back to Bug 1542181 Comment 16