Bug 1594521 Comment 19 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Right, so I think there are a few things going on here:
1. The worker is getting created late in the shutdown process and racing the worker's RuntimeService's compulsory shutdown of all workers.  This is a but in the RuntimeService, bug 1594572.
2. Something is going wrong in IndexedDB/PBackground when IDB requests are issued against a dying PBackground.  I've filed bug 1597312 on this.
3. Either RemoteSettings or a RemoteSettings consumer is doing things well after shutdown has started, or not using shutdown blockers to avoid things it started before shutdown from bleeding into shutdown.

I'll focus on the 3rd point for the remainder of the comment here.  I do want to emphasize that I acknowledge both the first two as important and serious bugs; and as situations that workers and IndexedDB have historically lucked out about because content consumers are destroyed earlier in cleanup.  System JS, and in particular JSMs, present an interesting complication because their lifetime is basically forever, until the runtime service terminates the worker.  And all of this is because shutdown is super confusing and not really documented or owned in Firefox/Gecko.

(In reply to :Gijs (he/him) from comment #15)
> What's even less clear to me is how wallpaper-y it would be to make the remote settings module force-`terminate()` its worker at profile-before-change (and refuse to start it once that's happened).

It's not wallpaper-y for a system service to be aware of system shutdown.  When the runtime service forces workers to terminate, they are aborted without regard for what they are doing.  Ideally the owner of the worker would use an appropriate shutdown blocker and:
- Block shutdown if the operation is important.
- Abort any outstanding transactions if they are not.  (While IndexedDB will force-abort transactions at profile-before-change-qm, this is *very* late in the shutdown process, and allows a potentially huge amount of wasted work to happen which has its own overhead as any pending transactions are rolled back.)

Besides the improved shutdown performance, a major benefit for services that report telemetry need to be able to distinguish between unexpected errors and expected situations like shutdown.  For example, Activity Stream's telemetry reports a tremendous number of TRANSACTION_FAILED errors that occur in its use of IndexedDB.  Hopefully these are all happening at shutdown when QuotaManager shuts down at `profile-before-change-qm` and aborts all in-progress transactions, but it's not clear.


It's my hope that we can fix both the workers and IDB bug in this cycle, but I would strongly encourage adding a shutdown blocker, especially if the remote settings API wants to have invariants about its storage being reliable in the face of browser shutdown.
Right, so I think there are a few things going on here:
1. The worker is getting created late in the shutdown process and racing the worker's RuntimeService's compulsory shutdown of all workers.  This is a but in the RuntimeService, bug 1594572.
2. Something is going wrong in IndexedDB/PBackground when IDB requests are issued against a dying PBackground.  I've filed bug 1597312 on this.
3. Either RemoteSettings or a RemoteSettings consumer is doing things well after shutdown has started, or not using shutdown blockers to avoid things it started before shutdown from bleeding into shutdown.

I'll focus on the 3rd point for the remainder of the comment here.  I do want to emphasize that I acknowledge both the first two as important and serious bugs; and as situations that workers and IndexedDB have historically lucked out about because content consumers are destroyed earlier in cleanup.  System JS, and in particular JSMs, present an interesting complication because their lifetime is basically forever.  And ChromeWorkers as well, by default, at least until the runtime service terminates the worker.  And all of this is because shutdown is super confusing and not really documented or owned in Firefox/Gecko.

(In reply to :Gijs (he/him) from comment #15)
> What's even less clear to me is how wallpaper-y it would be to make the remote settings module force-`terminate()` its worker at profile-before-change (and refuse to start it once that's happened).

It's not wallpaper-y for a system service to be aware of system shutdown.  When the runtime service forces workers to terminate, they are aborted without regard for what they are doing.  Ideally the owner of the worker would use an appropriate shutdown blocker and:
- Block shutdown if the operation is important.
- Abort any outstanding transactions if they are not.  (While IndexedDB will force-abort transactions at profile-before-change-qm, this is *very* late in the shutdown process, and allows a potentially huge amount of wasted work to happen which has its own overhead as any pending transactions are rolled back.)

Besides the improved shutdown performance, a major benefit for services that report telemetry need to be able to distinguish between unexpected errors and expected situations like shutdown.  For example, Activity Stream's telemetry reports a tremendous number of TRANSACTION_FAILED errors that occur in its use of IndexedDB.  Hopefully these are all happening at shutdown when QuotaManager shuts down at `profile-before-change-qm` and aborts all in-progress transactions, but it's not clear.


It's my hope that we can fix both the workers and IDB bug in this cycle, but I would strongly encourage adding a shutdown blocker, especially if the remote settings API wants to have invariants about its storage being reliable in the face of browser shutdown.
Right, so I think there are a few things going on here:
1. The worker is getting created late in the shutdown process and racing the worker's RuntimeService's compulsory shutdown of all workers.  This is a bug in the RuntimeService, bug 1594572.
2. Something is going wrong in IndexedDB/PBackground when IDB requests are issued against a dying PBackground.  I've filed bug 1597312 on this.
3. Either RemoteSettings or a RemoteSettings consumer is doing things well after shutdown has started, or not using shutdown blockers to avoid things it started before shutdown from bleeding into shutdown.

I'll focus on the 3rd point for the remainder of the comment here.  I do want to emphasize that I acknowledge both the first two as important and serious bugs; and as situations that workers and IndexedDB have historically lucked out about because content consumers are destroyed earlier in cleanup.  System JS, and in particular JSMs, present an interesting complication because their lifetime is basically forever.  And ChromeWorkers as well, by default, at least until the runtime service terminates the worker.  And all of this is because shutdown is super confusing and not really documented or owned in Firefox/Gecko.

(In reply to :Gijs (he/him) from comment #15)
> What's even less clear to me is how wallpaper-y it would be to make the remote settings module force-`terminate()` its worker at profile-before-change (and refuse to start it once that's happened).

It's not wallpaper-y for a system service to be aware of system shutdown.  When the runtime service forces workers to terminate, they are aborted without regard for what they are doing.  Ideally the owner of the worker would use an appropriate shutdown blocker and:
- Block shutdown if the operation is important.
- Abort any outstanding transactions if they are not.  (While IndexedDB will force-abort transactions at profile-before-change-qm, this is *very* late in the shutdown process, and allows a potentially huge amount of wasted work to happen which has its own overhead as any pending transactions are rolled back.)

Besides the improved shutdown performance, a major benefit for services that report telemetry need to be able to distinguish between unexpected errors and expected situations like shutdown.  For example, Activity Stream's telemetry reports a tremendous number of TRANSACTION_FAILED errors that occur in its use of IndexedDB.  Hopefully these are all happening at shutdown when QuotaManager shuts down at `profile-before-change-qm` and aborts all in-progress transactions, but it's not clear.


It's my hope that we can fix both the workers and IDB bug in this cycle, but I would strongly encourage adding a shutdown blocker, especially if the remote settings API wants to have invariants about its storage being reliable in the face of browser shutdown.

Back to Bug 1594521 Comment 19