Closed
Bug 616414
Opened 14 years ago
Closed 14 years ago
"Conditional jump or move depends on uninitialised value(s)" at IPC::ParamTraits<PrefTuple>::Write (caused by ContentParent::Observe)
Categories
(Core :: Preferences: Backend, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
fennec | 2.0+ | --- |
People
(Reporter: cjones, Assigned: cjones)
References
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
cjones
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
Problem is at
if (!strcmp(aTopic, "nsPref:changed")) {
// We know prefs are ASCII here.
NS_LossyConvertUTF16toASCII strData(aData);
PrefTuple pref;
nsCOMPtr<nsIPrefServiceInternal> prefService =
do_GetService("@mozilla.org/preferences-service;1");
prefService->MirrorPreference(strData, &pref);
where the last stmt above can fail at
NS_IMETHODIMP nsPrefService::MirrorPreference(const nsACString& aPrefName,
PrefTuple *aPref)
{
PrefHashEntry *pref = pref_HashTableLookup(nsDependentCString(aPrefName).get());
if (!pref)
return NS_ERROR_NOT_AVAILABLE;
if |pref| doesn't exist. The only reasonable hypothesis I can think of for that failure mode is that |aPrefName| has been deleted, in which case ContentParent needs to send a message indicating that instead of uninitialized garbage.
Assignee | ||
Comment 1•14 years ago
|
||
Dan, do you happen to know off-hand why this might happen?
Assignee | ||
Comment 2•14 years ago
|
||
Looks like it's a pref being cleared.
(gdb) bt
#0 mozilla::dom::ContentParent::Observe (this=0x7fffe01290d0, aSubject=0x7748c0, aTopic=0x7ffff643d50e "nsPref:changed", aData=0x7fffffff6bb0) at /home/cjones/mozilla/mozilla-central/dom/ipc/ContentParent.cpp:366
#1 0x00007ffff45f1475 in nsPrefBranch::NotifyObserver (newpref=0xba6820 "gfx.color_management.force_srgb", data=0x7fffe012d2b0) at /home/cjones/mozilla/mozilla-central/modules/libpref/src/nsPrefBranch.cpp:731
#2 0x00007ffff45fab85 in pref_DoCallback (changed_pref=0xba6820 "gfx.color_management.force_srgb") at /home/cjones/mozilla/mozilla-central/modules/libpref/src/prefapi.cpp:970
#3 0x00007ffff45fa09b in PREF_ClearUserPref (pref_name=0xba6820 "gfx.color_management.force_srgb") at /home/cjones/mozilla/mozilla-central/modules/libpref/src/prefapi.cpp:634
#4 0x00007ffff45f07c0 in nsPrefBranch::ClearUserPref (this=0x7748c0, aPrefName=0xba6820 "gfx.color_management.force_srgb") at /home/cjones/mozilla/mozilla-central/modules/libpref/src/nsPrefBranch.cpp:491
#5 0x00007ffff45f7cba in nsPrefService::ClearUserPref (this=0x72ef40, aPrefName=0xba6820 "gfx.color_management.force_srgb") at /home/cjones/mozilla/mozilla-central/modules/libpref/src/nsPrefService.h:62
(gdb) printf "%s", PrintJSStack()
0 OnRefTestUnload() ["chrome://reftest/content/reftest.js":329]
prefs = [xpconnect wrapped nsIPrefBranch2 @ 0x10555f0 (native @ 0x72ef40)]
this = [object ChromeWindow @ 0xe67850 (native @ 0xa4b348)]
1 onunload(event = [object Event @ 0xcd90c0 (native @ 0xedc050)]) ["chrome://reftest/content/reftest.xul":1]
this = [object ChromeWindow @ 0xe67850 (native @ 0xa4b348)]
function OnRefTestUnload()
{
/* Clear the sRGB forcing pref to leave the profile as we found it. */
var prefs = Components.classes["@mozilla.org/preferences-service;1"].
getService(Components.interfaces.nsIPrefBranch2);
prefs.clearUserPref("gfx.color_management.force_srgb");
Me trying to post this comment was apparently the first hint of The Great Bugzilla Outage of '10, and during it I just about finished up a fix for clearing.
Assignee | ||
Comment 3•14 years ago
|
||
Assignee: nobody → jones.chris.g
Attachment #496449 -
Flags: review?(dwitte)
Assignee | ||
Comment 4•14 years ago
|
||
Before the patch, we get
child: TEST-INFO | (xpcshell/head.js) | test 1 pending\nchild: TEST-PASS | /home/cjones/mozilla/ff-dbg/_tests/xpcshell/modules/libpref/test/unit_ipc/test_observed_prefs.js | [run_test : 12] true == true\nchild: TEST-PASS | /home/cjones/mozilla/ff-dbg/_tests/xpcshell/modules/libpref/test/unit_ipc/test_observed_prefs.js | [run_test : 13] 23 == 23\nchild: TEST-PASS | /home/cjones/mozilla/ff-dbg/_tests/xpcshell/modules/libpref/test/unit_ipc/test_observed_prefs.js | [run_test : 14] hey == hey\nchild: TEST-INFO | (xpcshell/head.js) | test 1 finished\nchild: TEST-INFO | (xpcshell/head.js) | exiting test\nchild: TEST-PASS | (xpcshell/head.js) | 3 (+ 0) check(s) passed\n###!!! ASSERTION: Unknown type: 'Not Reached', file /home/cjones/mozilla/mozilla-central/modules/libpref/src/prefapi.cpp, line 328
UNKNOWN [/home/cjones/mozilla/ff-dbg/dist/bin/libxul.so +0x0093E438]
UNKNOWN [/home/cjones/mozilla/ff-dbg/dist/bin/libxul.so +0x0093A7AB]
parent: TEST-INFO | (xpcshell/head.js) | test 2 finished
UNKNOWN [/home/cjones/mozilla/ff-dbg/dist/bin/libxul.so +0x01D203C4]
UNKNOWN [/home/cjones/mozilla/ff-dbg/dist/bin/libxul.so +0x01E8C1ED]
&c
(Ugh, check-one reverted back to not unbuffering. Ah well.)
After
child: TEST-INFO | (xpcshell/head.js) | test 1 pending\nchild: TEST-PASS | /home/cjones/mozilla/ff-dbg/_tests/xpcshell/modules/libpref/test/unit_ipc/test_observed_prefs.js | [run_test : 12] true == true\nchild: TEST-PASS | /home/cjones/mozilla/ff-dbg/_tests/xpcshell/modules/libpref/test/unit_ipc/test_observed_prefs.js | [run_test : 13] 23 == 23\nchild: TEST-PASS | /home/cjones/mozilla/ff-dbg/_tests/xpcshell/modules/libpref/test/unit_ipc/test_observed_prefs.js | [run_test : 14] hey == hey\nchild: TEST-INFO | (xpcshell/head.js) | test 1 finished\nchild: TEST-INFO | (xpcshell/head.js) | exiting test\nchild: TEST-PASS | (xpcshell/head.js) | 3 (+ 0) check(s) passed\ntypein:TypeError: redeclaration of const _TEST_FILE
parent: TEST-INFO | (xpcshell/head.js) | test 2 finished
Oops, I didn't notice "TypeError: redeclaration of const _TEST_FILE" before pasting here. Any idea what that might be?
Attachment #496450 -
Flags: feedback?(dwitte)
Assignee | ||
Comment 5•14 years ago
|
||
Comment on attachment 496450 [details] [diff] [review]
Test
Hmm, I wonder if run_test_in_child() isn't set up to run multiple child scripts. That's OK, I guess I could modify the test (would be happy with suggestions).
Attachment #496450 -
Flags: feedback?(dwitte) → feedback?(jduell.mcbugs)
Comment 6•14 years ago
|
||
Comment on attachment 496449 [details] [diff] [review]
Notify content processes when a pref has been cleared
>diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp
>+ nsresult rv = prefService->MirrorPreference(strData, &pref);
>+ if (NS_SUCCEEDED(rv)) {
>+ // Pref was created, or previously existed and its value
>+ // changed.
>+ if (!SendPreferenceUpdate(pref))
>+ return NS_ERROR_NOT_AVAILABLE;
>+ } else {
>+ // Pref wasn't found. It was probably removed.
>+ if (!SendClearUserPreference(strData))
>+ return NS_ERROR_NOT_AVAILABLE;
>+ }
I'd rather be a bit more explicit here. Instead of trying MirrorPreference() and seeing if it works or not, how about:
if prefService->PrefHasUserValue(...) then
prefService->MirrorPreference(...) and assert success
SendPreferenceUpdate(...)
else
SendClearUserPreference(...)
>diff --git a/modules/libpref/public/nsIPrefService.idl b/modules/libpref/public/nsIPrefService.idl
>+ [noscript] void clearUserPreference(in ACString aPrefName);
Sigh. This is tragically similar in name to nsIPrefBranch.clearUserPref(). I don't think we can get around that, but maybe we could give it a slightly less similar name? clearContentPref perhaps?
What would be really awesome is if we provided a global function to get the nsPrefService* singleton itself, like we do in cookies, and then these internal methods were only available there. ;)
Anyway, r=dwitte with the name change.
Attachment #496449 -
Flags: review?(dwitte) → review+
Comment 7•14 years ago
|
||
Comment on attachment 496450 [details] [diff] [review]
Test
> run_test_in_child("test_observed_prefs.js");
>+
>+ pb.clearUserPref("Test.IPC.bool.new");
>+ run_test_in_child("test_cleared_prefs.js");
run_test_in_child takes an optional callback function (documented in head.js!) that runs in the parent when the code sent to the child has completed. So I'm guessing you may want to change your serial calls to run_test_in_child to instead have the first call take a callback that calls the second run_test_in_child.
(But no one AFAIK is actually using multiple run_test_in_child, so while this ought to work, it might not)
Note that since run_test_in_child returns "immediately" (possibly before code is run in child) if test_observed_prefs.js should run/complete before clearUserPref, you'll want to move the latter to the callback as well.
Assuming you can get that to work (how is it failing now?), +r
Attachment #496450 -
Flags: feedback?(jduell.mcbugs) → feedback+
Assignee | ||
Comment 8•14 years ago
|
||
Carrying over r+.
Attachment #496449 -
Attachment is obsolete: true
Attachment #496450 -
Attachment is obsolete: true
Attachment #500238 -
Flags: review+
Assignee | ||
Comment 9•14 years ago
|
||
Multiple run_test_in_child() is not going to work without a lot more effort, so I did something simpler.
Assignee | ||
Comment 10•14 years ago
|
||
Oops forgot to nom this.
Needs to block because there's a loss-of-functionality bug here, not propagating user-pref clears to the content process, and also because in the current code this bug results in garbage values being used in the content process.
tracking-fennec: --- → ?
Updated•14 years ago
|
tracking-fennec: ? → 2.0+
Assignee | ||
Comment 11•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/549be0618b0a
http://hg.mozilla.org/mozilla-central/rev/79c44d25229d
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•