Closed Bug 1012466 Opened 5 years ago Closed 5 years ago

exception on shutdown: "Exception calling provider shutdown", NS_ERROR_FAILURE from nsIObserverService.removeObserver from PreviousExperimentProvider.shutdown

Categories

(Firefox Health Report Graveyard :: Client: Desktop, defect)

defect
Not set

Tracking

(firefox30 unaffected, firefox31 affected, firefox32+ fixed, firefox33 fixed)

RESOLVED FIXED
Firefox 33
Tracking Status
firefox30 --- unaffected
firefox31 --- affected
firefox32 + fixed
firefox33 --- fixed

People

(Reporter: Gavin, Assigned: gfritzsche)

References

Details

Attachments

(4 files, 7 obsolete files)

2.97 KB, text/plain
Details
8.09 KB, patch
gfritzsche
: review+
Details | Diff | Splinter Review
5.29 KB, patch
benjamin
: review+
Details | Diff | Splinter Review
11.08 KB, patch
benjamin
: review+
Details | Diff | Splinter Review
I shut down a recent trunk build and got:

1400448613755	addons.manager	ERROR	Exception calling provider shutdown: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource:///modules/experiments/Experiments.jsm :: this.Experiments.PreviousExperimentProvider.prototype<.shutdown :: line 2091"  data: no] Stack trace: this.Experiments.PreviousExperimentProvider.prototype<.shutdown()@resource:///modules/experiments/Experiments.jsm:2091 < callProvider()@resource://gre/modules/AddonManager.jsm:194 < AMI_unregisterProvider()@resource://gre/modules/AddonManager.jsm:849 < AMP_unregisterProvider()@resource://gre/modules/AddonManager.jsm:2322 < Experiments.Experiments.prototype._unregisterWithAddonManager()@resource:///modules/experiments/Experiments.jsm:500 < Experiments.Experiments.prototype.uninit<()@resource:///modules/experiments/Experiments.jsm:446 < TaskImpl_run()@resource://gre/modules/Task.jsm:282 < TaskImpl_handleResultValue()@resource://gre/modules/Task.jsm:338 < TaskImpl_run()@resource://gre/modules/Task.jsm:290 < TaskImpl()@resource://gre/modules/Task.jsm:247 < createAsyncFunction/asyncFunction()@resource://gre/modules/Task.jsm:224 < Barrier.prototype<._wait()@resource://gre/modules/AsyncShutdown.jsm:552 < Barrier.prototype<.wait()@resource://gre/modules/AsyncShutdown.jsm:518 < Spinner.prototype.observe()@resource://gre/modules/AsyncShutdown.jsm:337 < <file:unknown>


Can't seem to easily reproduce.
http://hg.mozilla.org/mozilla-central/annotate/41a54c8add09/browser/experiments/Experiments.jsm#l2019

I wonder if we never postInit()ed it? I think I've seen this in mochitest-chrome logs also, although it's not fatal.
Component: Add-ons Manager → Client: Desktop
Product: Toolkit → Firefox Health Report
I'm wrong, it's here: http://hg.mozilla.org/mozilla-central/annotate/41a54c8add09/browser/experiments/Experiments.jsm#l2091
Summary: exception on shutdown: "Exception calling provider shutdown", NS_ERROR_FAILURE from nsIObserverService.removeObserver → exception on shutdown: "Exception calling provider shutdown", NS_ERROR_FAILURE from nsIObserverService.removeObserver from PreviousExperimentProvider.shutdown
With the async shutdown, i wonder if we can end up with the following order:
* AddonManager shuts down, calls PreviousExperimentProvider.shutdown()
* Async shutdown triggers Experiments.uninit() -> _unregisterWithAddonManager() -> AddonManagerPrivate.unregisterProvider() -> PreviousExperimentProvider.shutdown()
... as i don't see any protection against that in AM or Experiments.

Also, we end up not saving the cache to disk in this case; we should at the very least take care of that.
Flags: firefox-backlog+
Whiteboard: p=5
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #0)
> I shut down a recent trunk build and got:
[...]

Did this finish with a crash, btw?
Flags: needinfo?(gavin.sharp)
No.
Flags: needinfo?(gavin.sharp)
(In reply to Georg Fritzsche [:gfritzsche] from comment #3)
> With the async shutdown, i wonder if we can end up with the following order:
> * AddonManager shuts down, calls PreviousExperimentProvider.shutdown()
> * Async shutdown triggers Experiments.uninit() ->
> _unregisterWithAddonManager() -> AddonManagerPrivate.unregisterProvider() ->
> PreviousExperimentProvider.shutdown()

So, Felipe reproduces this consistently and it turns out we have a double PreviousExperimentProvider.shutdown() - one of them before the experiments manager shutdown.
------- Call stack for first call ----------
> this.Experiments.PreviousExperimentProvider.prototype<.shutdown@resource:///modules/experiments/Experiments.jsm:2095:7
> AMI_callProviders@resource://gre/modules/AddonManager.jsm:904:15
> AMI_shutdown@resource://gre/modules/AddonManager.jsm:942:7
> Barrier.prototype<._wait@resource://gre/modules/AsyncShutdown.jsm:565:15
> Barrier.prototype<.wait@resource://gre/modules/AsyncShutdown.jsm:531:5
> Spinner.prototype.observe@resource://gre/modules/AsyncShutdown.jsm:337:7

------- Call stack for second call ----------
> this.Experiments.PreviousExperimentProvider.prototype<.shutdown@resource:///modules/experiments/Experiments.jsm:2095:7
> callProvider@resource://gre/modules/AddonManager.jsm:194:5
> AMI_unregisterProvider@resource://gre/modules/AddonManager.jsm:849:7
> AMP_unregisterProvider@resource://gre/modules/AddonManager.jsm:2321:5
> Experiments.Experiments.prototype._unregisterWithAddonManager@resource:///modules/experiments/Experiments.jsm:502:7
> Experiments.Experiments.prototype.uninit<@resource:///modules/experiments/Experiments.jsm:448:7
> TaskImpl_run@resource://gre/modules/Task.jsm:282:13
> TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:338:7
> TaskImpl_run@resource://gre/modules/Task.jsm:290:11
> TaskImpl@resource://gre/modules/Task.jsm:247:3
> createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:224:7
> Barrier.prototype<._wait@resource://gre/modules/AsyncShutdown.jsm:565:15
> Barrier.prototype<.wait@resource://gre/modules/AsyncShutdown.jsm:531:5
> Spinner.prototype.observe@resource://gre/modules/AsyncShutdown.jsm:337:7
Thanks Felipe!

I think the cleanest solution here is to trigger Experiments._unregisterWithAddonManager() when the AddonManager shuts down.

However i don't see right now how to either
a) get notified of AddonManager shutdown, so we can unregister or to
b) tell AsyncShutdown to order Experiments shutdown before the AddonManager

Blair, any recommendations?
Flags: needinfo?(bmcbride)
OS: Mac OS X → All
There is now a mechanism for ordering shutdown between services:
http://dutherenverseauborddelatable.wordpress.com/2014/05/26/shutting-down-asynchronously-part-2/

AddonManager should publish an instance of AsyncShutdown.Barrier. That's most likely a few lines of change.
Ah, thanks, that looks good.
Blair, is that reasonable for you?
WFM. Although, exposed only on AddonManagerPrivate please.
Flags: needinfo?(bmcbride)
Duplicate of this bug: 1006771
Some more discussion is found in the duped bug 1006771.
(In reply to Blair McBride [:Unfocused] from comment #12)
> WFM. Although, exposed only on AddonManagerPrivate please.

Why private? These days, a number of modules expose shutdown barriers for synchronization with other modules: OS.File, FHR Storage, Sqlite.jsm, ...
Assignee: nobody → georg.fritzsche
Whiteboard: p=5 → p=5 [qa-]
(In reply to David Rajchenbach Teller [:Yoric] from comment #15)
> (In reply to Blair McBride [:Unfocused] from comment #12)
> > WFM. Although, exposed only on AddonManagerPrivate please.
> 
> Why private? These days, a number of modules expose shutdown barriers for
> synchronization with other modules: OS.File, FHR Storage, Sqlite.jsm, ...

Can you make a call on that Blair?
Flags: needinfo?(bmcbride)
Added to Iteration 33.2
Status: NEW → ASSIGNED
Iteration: --- → 33.2
Points: --- → 5
Whiteboard: p=5 [qa-] → [qa-]
(In reply to Georg Fritzsche [:gfritzsche] from comment #16)
> (In reply to David Rajchenbach Teller [:Yoric] from comment #15)
> > (In reply to Blair McBride [:Unfocused] from comment #12)
> > > WFM. Although, exposed only on AddonManagerPrivate please.
> > 
> > Why private? These days, a number of modules expose shutdown barriers for
> > synchronization with other modules: OS.File, FHR Storage, Sqlite.jsm, ...
> 
> Can you make a call on that Blair?

Fair enough. AddonManager then.
Flags: needinfo?(bmcbride)
Irving, hg blame tells me you were working on the AM shutdown. What do you think of this?
Attachment #8448700 - Flags: review?(irving)
Comment on attachment 8448700 [details] [diff] [review]
Add shutdown barrier for AddonManager clients

Review of attachment 8448700 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/AddonManager.jsm
@@ +746,2 @@
>        AsyncShutdown.profileBeforeChange.addBlocker("AddonManager: shutting down providers",
>                                                     this.shutdown.bind(this));

You might consider adding as third argument of `addBlocker` a function that returns the current shutdown state, as a JS object. This will help debugging if something fails during shutdown.

@@ +2806,5 @@
>      return AddonManagerInternal.escapeAddonURI(aAddon, aUri, aAppVersion);
> +  },
> +
> +  get shutdownBarrier() {
> +    return gShutdownBarrier.client;

For the moment, the convention used in other modules is to call this simply `shutdown`.
(In reply to David Rajchenbach Teller [:Yoric] from comment #22)
> ::: toolkit/mozapps/extensions/AddonManager.jsm
> @@ +746,2 @@
> >        AsyncShutdown.profileBeforeChange.addBlocker("AddonManager: shutting down providers",
> >                                                     this.shutdown.bind(this));
> 
> You might consider adding as third argument of `addBlocker` a function that
> returns the current shutdown state, as a JS object. This will help debugging
> if something fails during shutdown.

Hm, i'm not sure what information this would contain, can you expand on that?

> @@ +2806,5 @@
> > +  get shutdownBarrier() {
> > +    return gShutdownBarrier.client;
> 
> For the moment, the convention used in other modules is to call this simply
> `shutdown`.

There is already a shutdown method on AddonManagerInternal, so this seems like too much potential for confusion.
(In reply to Georg Fritzsche [:gfritzsche] from comment #23)
> Hm, i'm not sure what information this would contain, can you expand on that?

For Sqlite.jsm, it's the list of databases that are still open.

For OS.File, it's the latest messages sent and received from/to the worker, as well as status regarding the current stage of shutdown.

Here, I imagine the list of providers that haven't completed shutdown yet could be useful.

> 
> > @@ +2806,5 @@
> > > +  get shutdownBarrier() {
> > > +    return gShutdownBarrier.client;
> > 
> > For the moment, the convention used in other modules is to call this simply
> > `shutdown`.
> 
> There is already a shutdown method on AddonManagerInternal, so this seems
> like too much potential for confusion.

Fair enough.
Comment on attachment 8448701 [details] [diff] [review]
Use AddonManager shutdown barrier in Experiments

Review of attachment 8448701 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/experiments/test/xpcshell/test_api.js
@@ +1529,1 @@
>    restartManager();

Since you're in a task, you could use 'yield promiseRestartManager();' here (there's also promiseShutdownManager() for yielding until the manager shuts down).

The non-promise versions spin a nested event loop inside themselves, which can lead to surprises.
Comment on attachment 8448700 [details] [diff] [review]
Add shutdown barrier for AddonManager clients

Review of attachment 8448700 [details] [diff] [review]:
-----------------------------------------------------------------

One long IRC conversation with Unfocused later, I'm OK with adding AsyncShutdown.Blocker support to AddonManager, with a follow up bug to convert our existing support for provider.shutdown() methods to also use the Blocker.

If the Experiments provider is *only* shut down by the AddonManager blocker, Experiments.uninit() doesn't need to do AddonManagerPrivate.unregisterProvider() because AM automatically forgets all its registered providers when it shuts down.

::: toolkit/mozapps/extensions/AddonManager.jsm
@@ +947,5 @@
>      let shuttingDown = null;
>      if (gStarted) {
> +      shuttingDown = gShutdownBarrier.wait()
> +        .then(() => this.callProvidersAsync("shutdown"),
> +              err => logger.error("Failure during wait for shutdown barrier", err))

Put the error handler in a separate .then() directly after gShutdownBarrier.wait(). With the implementation here the callProvidersAsync("shutdown") won't be executed if gShutdownBarrier.wait() rejects, and we *do* want to execute it in that case.

@@ +2806,5 @@
>      return AddonManagerInternal.escapeAddonURI(aAddon, aUri, aAppVersion);
> +  },
> +
> +  get shutdownBarrier() {
> +    return gShutdownBarrier.client;

That's a bit messy in this case because we already have a shutdown() method, though it would be easy enough to rename that; it's not an external API.
Attachment #8448700 - Flags: review?(irving) → review-
(In reply to :Irving Reid from comment #26)
> If the Experiments provider is *only* shut down by the AddonManager blocker,
> Experiments.uninit() doesn't need to do
> AddonManagerPrivate.unregisterProvider() because AM automatically forgets
> all its registered providers when it shuts down.

We do at least use uninit() explicitly in tests. Is this a potential issue or can we leave it?
If you uninit() and init() the Experiments provider without restarting the Addon Manager, you need to be careful not to leave multiple shutdown blockers registered, and/or you need to correctly handle the case where the AM shuts down at a time when the Experiments provider isn't expecting it to.

For the XPI provider, in tests we always completely shut down the Addon Manager and unload/reload XPIProvider.jsm to simulate a browser restart as closely as possible.
Attachment #8448700 - Attachment is obsolete: true
Attachment #8450263 - Flags: review?(irving)
Comment on attachment 8448700 [details] [diff] [review]
Add shutdown barrier for AddonManager clients

Review of attachment 8448700 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/AddonManager.jsm
@@ +966,5 @@
>          this.typeListeners.splice(0, this.typeListeners.length);
>          for (let type in this.startupChanges)
>            delete this.startupChanges[type];
>          gStarted = false;
>          gStartupComplete = false;

+     gShutdownBarrier = null;

to make sure we release all registered blockers when the test harness shuts down the AddonManager to simulate browser restarts.
Attachment #8448700 - Attachment is obsolete: false
Attachment #8448701 - Attachment is obsolete: true
Attachment #8450269 - Flags: review?(irving)
Attachment #8450269 - Flags: review?(irving)
Attachment #8448700 - Attachment is obsolete: true
Attachment #8450263 - Attachment is obsolete: true
Attachment #8450263 - Flags: review?(irving)
Attachment #8450275 - Flags: review?(irving)
Comment on attachment 8450275 [details] [diff] [review]
Part 1: Add shutdown barrier for AddonManager clients

Review of attachment 8450275 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/AddonManager.jsm
@@ +2808,5 @@
> +  },
> +
> +  get shutdown() {
> +    if (!gShutdownBarrier) {
> +      return null;

Side-note: This is needed to avoid test-failures here: http://hg.mozilla.org/mozilla-central/annotate/ac6960197eb6/toolkit/mozapps/extensions/test/xpcshell/test_shutdown.js#l25
Comment on attachment 8450275 [details] [diff] [review]
Part 1: Add shutdown barrier for AddonManager clients

Review of attachment 8450275 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/AddonManager.jsm
@@ +931,5 @@
>     * up everything in order for automated tests to fake restarts.
>     * @return Promise{null} that resolves when all providers and dependent modules
>     *                       have finished shutting down
>     */
> +  shutdownManager: function AMI_shutdown() {

we don't need the function name any more, now that JS infers it from the object & attribute names, but don't rev the patch just to change that.

@@ +2808,5 @@
> +  },
> +
> +  get shutdown() {
> +    if (!gShutdownBarrier) {
> +      return null;

test_shutdown.js drives me crazy - I always end up having to rev patches because it fails on something I added.

The right fix is probably to add shutdown to the IGNORE list, and check IGNORE before 'typeof != function' at lines 25-28 and 42-45 of test_shutdown.js, but I can live with this as is.
Attachment #8450275 - Flags: review?(irving) → review+
Iteration: 33.2 → 33.3
QA Whiteboard: [qa-]
Whiteboard: [qa-]
Duplicate of this bug: 1024183
Attached patch Part 3: Test overhaul (obsolete) — Splinter Review
Attachment #8450271 - Attachment is obsolete: true
The above patches are mostly finished, but currently blocked by at least one test issue in Part 3 that i'm too tired today to resolve:

> TEST-UNEXPECTED-FAIL | [...]/obj/_tests/xpcshell/browser/experiments/test/xpcshell/test_api.js | getActiveExperimentID should return the active experiment1 - null == "test-experiment-1@tests.mozilla.org" - See following stack:
>    [...]/obj/_tests/xpcshell/browser/experiments/test/xpcshell/test_api.js:test_getActiveExperimentID:348
...


This is proceeded by apparently a successful experiment activation, including the addon install, but then we fail to get the addon:

> 0:04.88 1404836201629	Browser.Experiments.Experiments	TRACE	ExperimentEntry #2::_installAddon() - install ended for test-experiment-1@tests.mozilla.org
> 0:04.88 1404836201630	Browser.Experiments.Experiments	TRACE	ExperimentEntry #2::Add-on is disabled. Enabling.
> 0:04.88 1404836201631	addons.xpi-utils	DEBUG	Updating active state for add-on test-experiment-1@tests.mozilla.org to true
> 0:04.89 1404836201631	DeferredSave.extensions.json	DEBUG	Save changes
> 0:04.89 1404836201631	addons.xpi	DEBUG	Registering manifest for /var/folders/tp/mgx9wlds46j6q3ck3xhjzm2w0000gn/T/tmphVU72e/extensions/test-experiment-1@tests.mozilla.org.xpi
> 0:04.89 1404836201631	addons.xpi	WARN	Add-on test-experiment-1@tests.mozilla.org is missing bootstrap method startup
> 0:04.89 1404836201632	Browser.Experiments.Experiments	INFO	ExperimentEntry #2::onEnabled() for test-experiment-1@tests.mozilla.org
> 0:04.89 1404836201632	addons.xpi	DEBUG	Registering manifest for /var/folders/tp/mgx9wlds46j6q3ck3xhjzm2w0000gn/T/tmphVU72e/extensions/test-experiment-1@tests.mozilla.org.xpi
> 0:04.89 1404836201632	addons.xpi	WARN	Add-on test-experiment-1@tests.mozilla.org is missing bootstrap method startup
> 0:04.89 1404836201632	DeferredSave.extensions.json	DEBUG	Starting timer
> 0:04.89 1404836201638	addons.xpi	DEBUG	removeTemporaryFile: http://localhost:63323/data/experiment-1.xpi removing temp file /Users/gfritzsche/Library/Caches/TemporaryItems/tmp-zoi.xpi
> 0:04.89 1404836201639	Browser.Experiments.Experiments	ERROR	Experiments #1::evaluateExperiments() - Unable to start experiment: Could not obtain add-on for experiment that should be enabled.
> 0:04.89 1404836201639	Browser.Experiments.Experiments	TRACE	ExperimentEntry #2::reconcileAddonState()
> 0:04.89 1404836201640	Browser.Experiments.Experiments	TRACE	ExperimentEntry #2::reconcileAddonState() - Inactive experiment has no add-on. Doing nothing.
Finally found the (test-only) issue, this should be good now.

https://tbpl.mozilla.org/?tree=Try&rev=f228a901623f
Attachment #8452410 - Attachment is obsolete: true
Attachment #8453029 - Flags: review?(benjamin)
Attachment #8452408 - Flags: review?(benjamin)
Attachment #8452408 - Flags: review?(benjamin) → review+
Attachment #8453029 - Flags: review?(benjamin) → review+
Try run from comment 42 is green.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4e67f3f51610
https://hg.mozilla.org/mozilla-central/rev/100fce3292d4
https://hg.mozilla.org/mozilla-central/rev/eba5f233f01b
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 33
Setting NEEDINFO for gfritzsche: I think we want an aurora uplift for this, but I'm not sure about risk assessment.
Flags: needinfo?(georg.fritzsche)
Comment on attachment 8452407 [details] [diff] [review]
Part 1: Add shutdown barrier for AddonManager clients

Approval Request Comment
[Feature/regressing bug #]: Telemetry experiments.
[User impact if declined]: Exception on shutdown, possibly out-of-sync experiments data.
[Describe test coverage new/current, TBPL]: baked on mc for a bit, automatic test-coverage not feasible.
[Risks and why]: Leaning to medium due to an addon manager change. It is well understood though and should be ok given proper bake-time.
[String/UUID change made/needed]: None.
Attachment #8452407 - Flags: approval-mozilla-aurora?
Flags: needinfo?(georg.fritzsche)
Comment on attachment 8452408 [details] [diff] [review]
Part 2: Use AddonManager shutdown barrier in Experiments

Approval Request Comment: See comment 47.
Attachment #8452408 - Flags: approval-mozilla-aurora?
Comment on attachment 8453029 [details] [diff] [review]
Part 3: Test overhaul

Approval Request Comment: See comment 47.
Attachment #8453029 - Flags: approval-mozilla-aurora?
(In reply to Georg Fritzsche [:gfritzsche] [away July 10-14] from comment #47)
> Comment on attachment 8452407 [details] [diff] [review]
> Part 1: Add shutdown barrier for AddonManager clients
> 
> Approval Request Comment
> [Feature/regressing bug #]: Telemetry experiments.
What is the oldest release that is impacted?

> [User impact if declined]: Exception on shutdown, possibly out-of-sync
> experiments data.
I take it from the early comments that this bug impacts all users not just those with an active experiment. Can you confirm?

> [Describe test coverage new/current, TBPL]: baked on mc for a bit, automatic
> test-coverage not feasible.
Was this shutdown crash 100% reproducible? How have you confirmed that the issue has been addressed?

> [Risks and why]: Leaning to medium due to an addon manager change. It is
> well understood though and should be ok given proper bake-time.
We're near the end of the Aurora 32 cycle. What would you say is proper bake time in this case?
Flags: needinfo?(georg.fritzsche)
(In reply to Lawrence Mandel [:lmandel] from comment #50)
> (In reply to Georg Fritzsche [:gfritzsche] [away July 10-14] from comment
> #47)
> > Comment on attachment 8452407 [details] [diff] [review]
> > Part 1: Add shutdown barrier for AddonManager clients
> > 
> > Approval Request Comment
> > [Feature/regressing bug #]: Telemetry experiments.
> What is the oldest release that is impacted?

Firefox 31, which is where we enabled experiments.

> > [User impact if declined]: Exception on shutdown, possibly out-of-sync
> > experiments data.
> I take it from the early comments that this bug impacts all users not just
> those with an active experiment. Can you confirm?

Yes, that is correct.

> > [Describe test coverage new/current, TBPL]: baked on mc for a bit, automatic
> > test-coverage not feasible.
> Was this shutdown crash 100% reproducible? How have you confirmed that the
> issue has been addressed?

It was not 100% reproducible, but confirmed by an affected user.

> > [Risks and why]: Leaning to medium due to an addon manager change. It is
> > well understood though and should be ok given proper bake-time.
> We're near the end of the Aurora 32 cycle. What would you say is proper bake
> time in this case?

I'm not 100% sure on an optimal bake-time, but personally i'd expect us to be free of major issues and fine for Aurora after 4-5 days.
Flags: needinfo?(georg.fritzsche)
Judging from Yorics shutdown dashboard (which may not be bug-free yet though), we probably fixed the experiments shutdown issue (bug 1012466) on 33 and can have a guess at numbers (with a sample size of 500):
http://yoric.github.io/are-we-shutting-down-yet-/?sample_size=500#Experiments.jsm%20shutdown

In the samples i checked, no recent 33 build ids are present.
Comment on attachment 8452407 [details] [diff] [review]
Part 1: Add shutdown barrier for AddonManager clients

Aurora+

Can we use Yoric's dashboard to confirm that this is fixed on Aurora?
Attachment #8452407 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8452408 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8453029 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Lawrence Mandel [:lmandel] from comment #53)
> Can we use Yoric's dashboard to confirm that this is fixed on Aurora?

It has a limited sample-size, but we can go for "probably fixed" as above.
Blocks: 1012924
(In reply to Georg Fritzsche [:gfritzsche] [away July 10-14] from comment #56)
> Backout while i take a bit to fix test failures:

... and the push with failures:
https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=e776a6a21d1c
A quick inspection of two days back showed no reports from after the landing of the patch here:
http://yoric.github.io/are-we-shutting-down-yet-/?sample_size=500&version=Firefox+32.0a2#Experiments.jsm%20shutdown

The previous caveats still apply though (caveats: bounded sample size, dashboard not guaranteed to be bug free).
(In reply to Georg Fritzsche [:gfritzsche] from comment #59)
> A quick inspection of two days back showed no reports from after the landing
> of the patch here

Awesome. I guess the real verification will be 32.0b1 shipping to the beta audience (search experiment will still be ongoing there, right?) which should then also not suffer from the crash. Does that sound correct?
Duplicate of this bug: 1007761
Depends on: 1071702
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in before you can comment on or make changes to this bug.