832677 - Telemetry uses preferences off the main thread (again)

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

ConstructTelemetryVFS gets a pref.  If it's called off the main thread, say because the storage service is being initialized off the main thread, lots of sadness happens.

This happens when running TestSettingsAPI.

Comment 1

[Marco Bonardo [:mak]]

ConstructTelemetryVFS() is called by Service::initialize, should probably be moved to ServiceMainThreadInitializer and likely a MOZ_ASSERT to check for mainthread in the constructor would be useful (related to that, why doesn't Preferences have any?)

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Marco Bonardo [:mak] from comment #1)
> ConstructTelemetryVFS() is called by Service::initialize, should probably be
> moved to ServiceMainThreadInitializer and likely a MOZ_ASSERT to check for
> mainthread in the constructor would be useful (related to that, why doesn't
> Preferences have any?)

Because people keep adding new off-main-thread uses before Bug 619487 can be fixed.

Comment 3

[Nathan Froyd [:froydnj]]

I talked with khuey this morning and he says this is the last thing blocking prefs-assert-off-the-main-thread.

AFAICS, however, we can't move the pref accessing in TelemetryVFS off the main thread until bug 836493 is fixed.

We *could* move the pref accessing to mozStorageService's main-thread initialization bits.  But that means we couldn't safely construct the TelemetryVFS until that point, too.  And if we somehow connected to databases before that main-thread initialization completed, we wouldn't be using TelemetryVFS.  And because TelemetryVFS handles database quotas, among other things...well, the failure modes are not pretty.

So that's what we're waiting on here.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I figured out a work around, so this doesn't block bug 619487.  We should still fix this though.

Comment 5

[Nathan Froyd [:froydnj]]

I think bug 836493 has effectively solved this: constructing TelemetryVFS only happens on the first call to Service::initialize, and that call now always happens on the main thread.  Mak, do you agree?

Comment 6

[Marco Bonardo [:mak]]

yep!