Assertion failure: _mOwningThread.GetThread() == PR_GetCurrentThread() (ValueObserver not thread-safe), at /Volumes/2mac/gaia/isrc/modules/libpref/src/Preferences.cpp:131

RESOLVED FIXED in mozilla19

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gwagner, Assigned: jgilbert)

Tracking

unspecified
mozilla19
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 3 obsolete attachments)

(Reporter)

Description

5 years ago
I see this during shutdown of the b2g desktop build.
(Reporter)

Updated

5 years ago
Assignee: nobody → anygregor
(Reporter)

Comment 1

5 years ago
Created attachment 667171 [details] [diff] [review]
patch
(Reporter)

Comment 2

5 years ago
The backtrace:
http://pastebin.mozilla.org/1855092
(Reporter)

Comment 3

5 years ago
Comment on attachment 667171 [details] [diff] [review]
patch

This fixes the problem but I am not 100% sure if this is the right fix.
Attachment #667171 - Flags: review?(roc)
Comment on attachment 667171 [details] [diff] [review]
patch

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

This is absolutely not the right fix. Preferences are not threadsafe and simply declaring them to be won't fix anything, it will just hide bugs.

It looks like something added a preference observer off the main thread. It needs to not do that.

nsPrefBranch::AddObserver and nsPrefBranch::RemoveObserver should assert NS_IsMainThread. That will let you catch this bug when it is actually introduced.
Attachment #667171 - Flags: review?(roc) → review-
Created attachment 676189 [details]
Assertion stack trace

This is the first place where AddObserver is called from the main thread.
Created attachment 676192 [details] [diff] [review]
Abort on access from the main thread

This is the patch I used to get the log above. I made it abort because I was getting a bazillion assertions just waiting for the homescreen to appear.
Your patch is backwards, you want to abort if you're *not* on the main thread.
Created attachment 676214 [details] [diff] [review]
Abort on access off the main thread

Doh! This is the version that aborts on access off the main thread.
Attachment #676192 - Attachment is obsolete: true
Created attachment 676215 [details]
Assertion stack trace

...and this is the log from running desktop B2G with the above patch.
Attachment #676189 - Attachment is obsolete: true
Component: General → Graphics
Product: Boot2Gecko → Core
mozilla::gl::GLContext::CanUploadNonPowerOfTwo should absolutely not be calling AddBoolVarCache when off the main thread!!!
Assignee: anygregor → jgilbert
(Assignee)

Comment 11

5 years ago
Created attachment 676876 [details] [diff] [review]
Do caching setup in GLContext::Init
Attachment #676876 - Flags: review?(bjacob)
Attachment #676876 - Flags: feedback?(roc)
Attachment #676876 - Flags: feedback?(roc) → feedback+
Comment on attachment 676876 [details] [diff] [review]
Do caching setup in GLContext::Init

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

This seems to rely on the assumption that InitWithPrefix is only called on the main thread, which AFAIU isn't the case.

The approach we've used in previous similar bugs was to move such must-happen-on-main-thread initialization things to gfxPlatform. See e.g. how at the beginning of InitWithPrefix we query gfxPlatform for WorkAroundDriverBugs(). Indeed gfxPlatform initialization at least is guaranteed to happen on the main thread.
Attachment #676876 - Flags: review?(bjacob) → review-
See this patch -- with you reviewed ;-)
https://bugzilla.mozilla.org/attachment.cgi?id=613137&action=diff
Side note: it would be nice if we could have read-only, no-guarantees-in-case-of-race, access to preferences off the main thread. Not having that is causing many such discussions, and is causing us to move more things to the main thread at startup for no real good reason.
Created attachment 678208 [details] [diff] [review]
Do caching setup in GLContext::Init
Attachment #676876 - Attachment is obsolete: true
Attachment #678208 - Flags: review?(bjacob)
Comment on attachment 678208 [details] [diff] [review]
Do caching setup in GLContext::Init

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

::: gfx/gl/GLContext.cpp
@@ +641,2 @@
>  
>      if (!sPowerOfTwoPrefCached) {

Seems like this CacheCanUploadNPOT should only ever get called once, from gfxPlatform::Init--->GLContext::PlatformSetup. So you could consider either removing this if(), or replacing it by an assertion confirming that we area only doing this once.
Attachment #678208 - Flags: review?(bjacob) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d54c207490e
https://hg.mozilla.org/mozilla-central/rev/6d54c207490e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19

Updated

5 years ago
Duplicate of this bug: 809363
You need to log in before you can comment on or make changes to this bug.