Closed
Bug 842611
Opened 12 years ago
Closed 12 years ago
Don't let nsOfflineCacheDevice::Init() create mozStorageService on non-main thread
Categories
(Core :: Networking: Cache, defect)
Core
Networking: Cache
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: mayhemer, Assigned: mayhemer)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
michal
:
review+
mayhemer
:
checkin+
|
Details | Diff | Splinter Review |
Make sure this initializes mozStorageService on the main thread.
Assignee | ||
Comment 1•12 years ago
|
||
- just an added assertion
try: https://tbpl.mozilla.org/?tree=Try&rev=5e93e389ebb6
Attachment #715551 -
Flags: review?(jduell.mcbugs)
Updated•12 years ago
|
Attachment #715551 -
Flags: review?(jduell.mcbugs) → review+
Assignee | ||
Comment 2•12 years ago
|
||
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
Attachment #715551 -
Flags: review+ → review-
Assignee | ||
Comment 3•12 years ago
|
||
Looks promising: https://tbpl.mozilla.org/?tree=Try&rev=9cb1c6fbf590
Assignee | ||
Comment 4•12 years ago
|
||
- 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
Attachment #715551 -
Attachment is obsolete: true
Attachment #733390 -
Flags: review?(michal.novotny)
Assignee | ||
Comment 5•12 years ago
|
||
And for reference, here is MOZ_ASSERT(IsMainThread) check try run (completely broken): https://tbpl.mozilla.org/?tree=Try&rev=df9a1a5fe944
Assignee | ||
Updated•12 years ago
|
Summary: Add MOZ_ASSERT(NS_IsMainThread()) to nsOfflineCacheDevice::Init() → Don't let nsOfflineCacheDevice::Init() create mozStorageService on non-main thread
Comment 6•12 years ago
|
||
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.
Attachment #733390 -
Flags: review?(michal.novotny) → review+
Assignee | ||
Comment 7•12 years ago
|
||
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
Assignee | ||
Comment 8•12 years ago
|
||
Comment on attachment 733390 [details] [diff] [review]
v2
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3df7c77c465
Attachment #733390 -
Flags: checkin+
Comment 9•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in
before you can comment on or make changes to this bug.
Description
•