842611 - Don't let nsOfflineCacheDevice::Init() create mozStorageService on non-main thread

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Honza Bambas (:mayhemer)]

Make sure this initializes mozStorageService on the main thread.

Comment 1

[Honza Bambas (:mayhemer)]

Attachment: v1

- just an added assertion

try: https://tbpl.mozilla.org/?tree=Try&rev=5e93e389ebb6

Comment 2

[Honza Bambas (:mayhemer)]

Comment on attachment 715551 [details] [diff] [review]
v1

r- because of try run failures.  It however doesn't mean Init() is not called subsequently when the DB has already been initialized.

Try'ing a mod of the patch: https://tbpl.mozilla.org/?tree=Try&rev=02e6ee31092a

Comment 3

[Honza Bambas (:mayhemer)]

Looks promising: https://tbpl.mozilla.org/?tree=Try&rev=9cb1c6fbf590

Comment 4

[Honza Bambas (:mayhemer)]

Attachment: v2

- based on fact that nsCacheService::Init() is called on main thread only and is precondition to use of any device
- nsCacheService::Init() instantiates and holds the storage service
- nsOfflineCacheDevice::Init() converted to a not-reached method
- non-virtual nsOfflineCacheDevice::InitWithSqlite(mozIStorageService*) is used to initialize offline cache device now
- I rather made nsOfflineCacheDevice::Init() not-reached to be sure no-one can call it before nsCacheService::Init() has successfully obtained storage service on the main thread first
- only way to completely prevent of call to GetService(mozIStorageService) from non-main thread

Full try run now: https://tbpl.mozilla.org/?tree=Try&rev=2bad208d30d7

Comment 5

[Honza Bambas (:mayhemer)]

And for reference, here is MOZ_ASSERT(IsMainThread) check try run (completely broken): https://tbpl.mozilla.org/?tree=Try&rev=df9a1a5fe944

Comment 6

[Michal Novotny [:michal]]

Comment on attachment 733390 [details] [diff] [review]
v2

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

::: netwerk/cache/nsCacheService.cpp
@@ +1719,2 @@
>      if (NS_FAILED(rv)) {
>          CACHE_LOG_DEBUG(("OfflineDevice->Init() failed (0x%.8x)\n", rv));

The comment should be updated.

Comment 7

[Honza Bambas (:mayhemer)]

Comment on attachment 733390 [details] [diff] [review]
v2

https://tbpl.mozilla.org/?tree=Try&rev=2bad208d30d7 (JP failures come from the parent changeset), repush:
https://tbpl.mozilla.org/?tree=Try&rev=9914cbbdca7b

Comment 8

[Honza Bambas (:mayhemer)]

Comment on attachment 733390 [details] [diff] [review]
v2

https://hg.mozilla.org/integration/mozilla-inbound/rev/f3df7c77c465

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f3df7c77c465