From log in bug 845842: 1361986826492 Services.HealthReport.HealthReporter INFO Request to shut down. 1361986826492 Services.HealthReport.HealthReporter WARN Collector is in progress of initializing. Waiting to finish. 1361986876724 Services.DataReporting.Policy INFO Requesting data submission. Will expire at Wed Feb 27 2013 12:51:16 GMT-0500 (EST) 1361986876728 Services.Metrics.Collector INFO Initializing provider with storage: org.mozilla.appInfo While it should be a small window, we shouldn't honor submission requests once shutdown has been initiated!

Is this going to be sensitive to the order we run shutdown handlers? Is it OK for other modules to request data submissions while they are shutting down? If so, FHR will need to wait for everything else to shut down before it closes the gate.

This should do it. I think the next time we significantly refactor this file we should use distinct types to model the different service states instead of tracking state within one type. e.g. let hr = getHealthReporter(...) -> UninitializedHealthReporter hr.init().then(function onInit(reporter) { ... }) -> InitializedHealthReporter Something to think about.

Comment on attachment 725454 [details] [diff] [review] Check initialized state, v1 [Approval Request Comment] Bug caused by (feature/regressing bug #): FHR initial landing. User impact if declined: FHR could start doing work during shutdown. This would delay shutdown and create a bad user experience, especially on slow machines. Testing completed (on m-c, etc.): It's been on m-c for a few days. We added tests. Risk to taking this patch (and alternatives if risky): low risk String or UUID changes made by this patch: None This is a P1 FHR bug, which means it's a beta blocker. If we don't uplift this to 21, it may delay FHR by a release.

Comment on attachment 725454 [details] [diff] [review] Check initialized state, v1 low risk, P1 requirement for FHR and has baked on m-c for a few days.Approving for uplift . Thanks for the adding the tests here :)

Considering this is a Beta blocker, is there something I can do to confirm this is working as expected?

I think it would be very difficult to verify this manually. We have test coverage in the patch which I think is sufficient. If this bug were causing issues in the wild, we'd hear reports of shutdown hangs.

(In reply to Gregory Szorc [:gps] from comment #9) > I think it would be very difficult to verify this manually. We have test > coverage in the patch which I think is sufficient. > > If this bug were causing issues in the wild, we'd hear reports of shutdown > hangs. Okay thanks Gregory. Should this be marked in-testsuite?

Yup.