62935 - Crash when pref with multiple observers is changed.

Status: VERIFIED DUPLICATE of bug 65616

(Core :: Preferences: Backend, defect, P3)

Description

[[e-mail]]

I put this code in an XBL binding to register for preference changes:

function PrefObserver( pref, element )
{
  this.pref = pref;
  this.element = element;
  this.prefSvc = Components.classes["@mozilla.org/preferences;1"].getService(
Components.interfaces.nsIPref );
}
PrefObserver.prototype.Observe = function ( obj, topic, pref )
{
  dump("observation\n");
}
PrefObserver.prototype.Add = function()
{
  this.prefSvc.addObserver( this.pref, this );
}
PrefObserver.prototype.Remove = function()
{
  this.prefSvc.removeObserver( this.pref, this );
}
this.observer = new PrefObserver( "mail.pane_config", this );
this.observer.Add();

[observer is a property defined on the binding]

Changing the preference causes Mozilla to crash.

Comment 1

[[e-mail]]

Mozilla {Build ID: 2000121404}

MOZILLA caused an invalid page fault in module XPPREF32.DLL at 0157:60a92966.
Registers:
EAX=0e9d9860 CS=0157 EIP=60a92966 EFLGS=00010216
EBX=00000040 SS=015f ESP=0068d734 EBP=0068d7d0
ECX=00000001 DS=015f ESI=0e9d9890 FS=5387
EDX=0e9dfd6c ES=015f EDI=00000000 GS=0000
Bytes at CS:EIP:
ff 51 04 53 56 57 8d 8d 68 ff ff ff ff 75 08 ff 
Stack dump:
0e9d9860 00000000 0068d778 60a3e8ca 0eec0370 0068d671 0eeb6554 0068d770 
600a44cb 010a8d30 0eec0370 0eeb655c 0068d7b0 0068d7e8 0eec0370 0068d78c 
Stack trace:
XPPREF32! 60a9212f()
XPPREF32! 60a9152f()
XPC3250! 60a3e051()
XPC3250! 60a3e8ca()
JS3250! 60b470cd()
JS3250! 60b4b9ad()
JS3250! 60b4710b()
JS3250! 60b47380()
JS3250! 60b3454e()
JSDOM! 60b8f817()
JSDOM! 60ba78ca()
GKHTML! 602c1a75()
GKHTML! 602c297d()
RDF! 607c0bea()
GKHTML! 60207f2f()
GKHTML! 60296a61()
GKHTML! 6029698f()
GKHTML! 60207ea5()
GKHTML! 60207db0()
GKHTML! 602c6127()
GKHTML! 602c54b1()
GKHTML! 60207ecd()
GKHTML! 60207d29()
GKVIEW! 603e1406()
GKVIEW! 603ea118()
GKVIEW! 603e1dad()
GKWIDGET! 60ae5a58()
GKWIDGET! 60ae84ca()
GKWIDGET! 60ae87ff()
GKWIDGET! 60ae5ee9()
KERNEL32! bff735d9()
KERNEL32! bff9222f()
Disassembly:
XPPREF32! 60a9212f()
60A92950 55                   push        ebp
60A92951 8B EC                mov         ebp,esp
60A92953 81 EC 98 00 00 00    sub         esp,98h
60A92959 8B 45 0C             mov         eax,dword ptr [ebp+0Ch]
60A9295C 85 C0                test        eax,eax
60A9295E 89 45 0C             mov         dword ptr [ebp+0Ch],eax
60A92961 74 06                je          60A92969
60A92963 8B 08                mov         ecx,dword ptr [eax]
60A92965 50                   push        eax
60A92966 FF 51 04             call        dword ptr [ecx+4]
60A92969 53                   push        ebx
60A9296A 56                   push        esi
60A9296B 57                   push        edi
60A9296C 8D 8D 68 FF FF FF    lea         ecx,dword ptr [ebp-98h]
60A92972 FF 75 08             push        dword ptr [ebp+8]
60A92975 FF 15 B4 51 A9 60    call        dword ptr ds:[60A951B4h]
60A9297B 8B 75 0C             mov         esi,dword ptr [ebp+0Ch]
60A9297E 8B C8                mov         ecx,eax
60A92980 8B FE                mov         edi,esi
60A92982 8B 1E                mov         ebx,dword ptr [esi]
60A92984 FF 15 C4 51 A9 60    call        dword ptr ds:[60A951C4h]
60A9298A 50                   push        eax
60A9298B 68 B8 74 A9 60       push        60A974B8h
60A92990 57                   push        edi
60A92991 56                   push        esi
60A92992 FF 53 0C             call        dword ptr [ebx+0Ch]
60A92995 8D 8D 68 FF FF FF    lea         ecx,dword ptr [ebp-98h]
60A9299B FF 15 8C 51 A9 60    call        dword ptr ds:[60A9518Ch]
60A929A1 8D 4D 0C             lea         ecx,dword ptr [ebp+0Ch]
60A929A4 E8 73 19 00 00       call        60A9431C
60A929A9 5F                   pop         edi
60A929AA 5E                   pop         esi
60A929AB 33 C0                xor         eax,eax
60A929AD 5B                   pop         ebx
60A929AE C9                   leave
60A929AF C3                   ret
XPPREF32! 60a9152f()
60A9151E 6A 01                push        1
60A91520 6A 40                push        40h
60A91522 FF 74 24 10          push        dword ptr [esp+10h]
60A91526 FF 74 24 10          push        dword ptr [esp+10h]
60A9152A E8 97 07 00 00       call        60A91CC6
60A9152F 83 C4 10             add         esp,10h
60A91532 C3                   ret

Comment 2

[[e-mail]]

Changing summary to reflect cause of bug.

Apparently Peter ``jag'' Annema will look into this later.

Comment 3

[Peter "jag" Annema]

So one problem is that nsPref.cpp doesn't support more than one observer per
preference:

http://lxr.mozilla.org/seamonkey/source/modules/libpref/src/nsPref.cpp#1234

Now, the mObservers hashtable is just a way of keeping at least one reference
alive (so you don't have to), the actual callbacks are done elsewhere. This code
seems to be running into a dangling pointer, suggesting we've got a
AddRef/Release imbalance. A quick hack adding |NS_IF_ADDREF(observer);| to
::AddObserver stops the crashing, but is not the right way to go ;-P

I'll see if I can come up with a simple testcase.

Comment 4

[Daniel Veditz [:dveditz]]

Alecf opened bug 65616 on himself and says a patch is forthcoming

*** This bug has been marked as a duplicate of 65616 ***

Comment 5

[sairuh (rarely reading bugmail)]

mass verification of duplicate bugs: to find all bugspam pertaining to this, set
your search string to "DuplicateBugsBelongInZahadum".

if you think this particular bug is *not* a duplicate, please provide a
compelling reason, as well as check a recent *trunk* build (on the appropriate
platform[s]), before reopening.