101294 - my build doesn't work, and preferences dereferences null

Status: RESOLVED FIXED

(Core :: Preferences: Backend, defect)

Description

[timeless]

###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().: 
'mRawPtr != 0', file ..\..\..\dist\include\nsCOMPtr.h, line 649
^now that's really useful...

it turns out that i'm asserting in ReadUserPrefs ...
#define NS_FORWARD_NSIPREFSERVICE(_to) \
  NS_IMETHOD ReadUserPrefs(nsIFile *aFile) { return _to ReadUserPrefs(aFile); } 
\
  NS_IMETHOD ResetPrefs(void) { return _to ResetPrefs(); } \
  NS_IMETHOD ResetUserPrefs(void) { return _to ResetUserPrefs(); } \
  NS_IMETHOD SavePrefFile(nsIFile *aFile) { return _to SavePrefFile(aFile); } \
  NS_IMETHOD GetBranch(const char *aPrefRoot, nsIPrefBranch **_retval) { return 
_to GetBranch(aPrefRoot, _retval); } \
  NS_IMETHOD GetDefaultBranch(const char *aPrefRoot, nsIPrefBranch **_retval) { 
return _to GetDefaultBranch(aPrefRoot, _retval); } 

it's kind of interesting, because

nsPref::nsPref()
//-----------------------------------------------------------------------------
-----------
{
  PR_AtomicIncrement(&g_InstanceCount);
  NS_INIT_REFCNT();

  mPrefService = do_GetService(NS_PREFSERVICE_CONTRACTID);

  if (mPrefService)
    mPrefService->GetDefaultBranch("", getter_AddRefs(mDefaultBranch));
}

we check to see if mPrefService is nonnull here^^ but we don't appear to check 
elsewhere. And we don't assert here, which is probably the only useful place to 
assert.

I have no idea why my pref service is null, i'm sure that's a very bad thing, 
and i know i can't run mozilla/winembed/viewer because of it.  So any help in 
fixing my overall problem would be useful, but at least can we add some useful 
asserts or protect the evil macro code?

the first fatal stack is:
nsCOMPtr<nsIPrefService>::operator->() line 649 + 34 bytes
nsPref::ReadUserPrefs(nsPref * const 0x006d9e40, nsIFile * 0x00000000) line 80 
+ 19 bytes
nsViewerApp::Initialize(nsViewerApp * const 0x006a67b0, int 1, char * * 
0x006252b0) line 326
main(int 1, char * * 0x006252b0) line 157
mainCRTStartup() line 338 + 17 bytes

somehow i also managed to get this stack:
nsCOMPtr<nsIPref>::operator->() line 649
COOKIE_RegisterPrefCallbacks() line 377 + 17 bytes
nsCookieService::Init() line 63
nsCookieServiceConstructor(nsISupports * 0x00000000, const nsID & {...}, void * 
* 0x0012fe44) line 41 + 125 bytes
nsGenericFactory::CreateInstance(nsGenericFactory * const 0x006d8320, 
nsISupports * 0x00000000, const nsID & {...}, void * * 0x0012fe44) line 59 + 21 
bytes
nsComponentManagerImpl::CreateInstance(nsComponentManagerImpl * const 
0x00627770, const nsID & {...}, nsISupports * 0x00000000, const nsID & {...}, 
void * * 0x0012fe44) line 1543 + 24 bytes
nsComponentManagerImpl::GetService(nsComponentManagerImpl * const 0x00627774, 
const nsID & {...}, const nsID & {...}, nsISupports * * 0x0012fedc, 
nsIShutdownListener * 0x00000000) line 1700 + 30 bytes
nsServiceManager::GetService(const nsID & {...}, const nsID & {...}, 
nsISupports * * 0x0012fedc, nsIShutdownListener * 0x00000000) line 3289
nsGetServiceByCID::operator()(const nsID & {...}, void * * 0x0012fedc) line 225 
+ 22 bytes
nsCOMPtr<nsICookieService>::assign_from_helper(const nsCOMPtr_helper & {...}, 
const nsID & {...}) line 971 + 18 bytes
nsCOMPtr<nsICookieService>::nsCOMPtr<nsICookieService>(const nsCOMPtr_helper & 
{...}) line 553
nsViewerApp::SetupRegistry(nsViewerApp * const 0x006a6500) line 263 + 30 bytes

Comment 1

[Brian Nesse (gone)]

Timeless, is this branch or trunk? The trunk hasn't changed recently. I had a
report of something similar on the branch after a check-in last week, but the
problem wasn't reproducible and resolved itself after a fresh code pull.

This is a prime example of why we need to move away from nsIPrefs.
nsPrefService::Init() throws an error if the prefs service fails to start up.
nsPref::nsPref() can not... thus we are open to this sort of thing.

One thing that might cause the prefService to fail to start up would be the
existence of the preference "general.config.filename" (likely set to
"netscape.cfg") in your all.js or some similar file with no corresponding .cfg
file. Another could be the existence of said config file as the "hash" algorithm
was changed recently. Either way, you probably shouldn't have this preference set.

The simplest way to identify your specific issue would probably be to breakpoint
nsPrefService::Init() and see who kicks back the error condition.

Comment 2

[timeless]

trunk - but there is at least something broken. xpcshell works so if you can
give me specific steps to test there, that'd be great. i'll set the breakpoint
and report sometime this evening. -- my build had lots of merge conflicts
including some in all.js which i suspected but i cleared those out.

Comment 3

[Brian Nesse (gone)]

nsPrefService::Init() [\mozilla\modules\libpref\src\nsPrefService.cpp line 113]
only does 3 things:

It calls PREF_INIT(). If this fails, there is something seriously wrong... no JS
context, bad files in the defaults/prefs directory, etc.

It calls readConfigFile(). This is the case I described previously. If the
failure is in here, it will likely fail near the top of the method. What you
should see here is it failing to get the preference "general.config.filename"
and returning NS_OK. If you get past that point, the preference is set somewhere.

It sets itself to observe profile changes from the observerService. This really
shouldn't fail, but it will return an error condition if it does.

Comment 4

[timeless]

msdev refused to set a breakpoint at that line or even function.  i suspect
therein lies the problem.  suggestions about what to rebuild or diff?

Comment 5

[Brian Nesse (gone)]

Hmm, well line 113 is the actual declaration of the function. Line 117 would be
the first executable line of code. But if you can't breakpoint anything in the
function, yes something is wrong.

I'd clobber the libpref module (it's only 6 files) and rebuild that (you might
toast the component registry also.)

Comment 6

[Brian Nesse (gone)]

timeless, are you still stuck on this?

I have added an assertion in the nsIPref constructor as you suggested... not
that it helps you any...

Comment 7

[timeless]

my build failed near nsAppFileLocationProvider.cpp which has suffered heavy
merge conflicts, after moving that out of the way, i got a build which sort of
works (growing windows fails).  I'll send the file or something like it around
later if i can get it building (it has evil requires stuff which doesn't work
these days...)

Comment 8

[Brian Nesse (gone)]

Reassigning to reporter because there is nothing identifiable for me to work on
here. timeless, if you still need help on this let me know.

Comment 9

[timeless]

Attachment: make sure that nsPref is never unhappy at construction time

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 183875 [details] [diff] [review]
make sure that nsPref is never unhappy at construction time

Looks OK as far as it goes, but what's up with the g_InstanceCount thing?

Doesn't NS_RELEASE() null out gInstance? So how does it get reused? If it's not
reused why bother with a static pointer to it?

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 183875 [details] [diff] [review]
make sure that nsPref is never unhappy at construction time

sr=dveditz

Comment 12

[Phil Ringnalda (:philor)]

timeless: so, this reviewed and super-reviewed patch from 2005 still applies (go barely-used modules); do you want to check it in, or did you abandon it when you realized there was something wrong with it?

Comment 13

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/mozilla-central/rev/908c5a39cdd3