749182 - crash in nsSessionStorageEntry::~nsSessionStorageEntry

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Scoobidiver (away)]

It first appeared in 15.0a1/20120426 and happens only with 64-bit builds. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=75c7378c87b6&tochange=cc5254f9825f
It's likely a regression from bug 746272.

Signature 	nsRefPtr<mozilla::DOMSVGAnimatedLengthList>::~nsRefPtr<mozilla::DOMSVGAnimatedLengthList>() | nsCOMPtr_base::~nsCOMPtr_base() | nsSessionStorageEntry::~nsSessionStorageEntry() More Reports Search
UUID	043b7217-d42d-4796-853f-1ff4e2120426
Date Processed	2012-04-26 13:59:38
Uptime	109
Last Crash	1.9 minutes before submission
Install Age	48.9 minutes since version was first installed.
Install Time	2012-04-26 13:10:31
Product	Firefox
Version	15.0a1
Build ID	20120426030504
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
Build Architecture	amd64
Build Architecture Info	family 6 model 23 stepping 10
Crash Reason	EXCEPTION_ACCESS_VIOLATION_EXEC
Crash Address	0x400010001
App Notes 	
AdapterVendorID: 0x10de, AdapterDeviceID: 0x0a22, AdapterSubsysID: 1141174b, AdapterDriverVersion: 8.17.12.9573
D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ 
EMCheckCompatibility	False	
Total Virtual Memory	8796092891136
Available Virtual Memory	8795386687488
System Memory Use Percentage	41
Available Page File	6224048128
Available Physical Memory	2502184960

Frame 	Module 	Signature 	Source
0 		@0x400010001 	
1 	xul.dll 	nsRefPtr<mozilla::DOMSVGAnimatedLengthList>::~nsRefPtr<mozilla::DOMSVGAnimatedLe 	obj-firefox/dist/include/nsAutoPtr.h:908
2 	xul.dll 	nsCOMPtr_base::~nsCOMPtr_base 	obj-firefox/dist/include/nsAutoPtr.h:908
3 	xul.dll 	nsSessionStorageEntry::~nsSessionStorageEntry 	dom/src/storage/nsDOMStorage.cpp:250
4 	xul.dll 	nsDOMStoragePersistentDB::RemoveKey 	dom/src/storage/nsDOMStoragePersistentDB.cpp:633
5 	xul.dll 	nsSessionStorageEntry::`scalar deleting destructor' 	
6 	xul.dll 	JS_DHashTableRawRemove 	js/src/jsdhash.cpp:714
7 	xul.dll 	DOMStorageImpl::RemoveValue 	dom/src/storage/nsDOMStorage.cpp:1269
8 	xul.dll 	JS_FrameIterator 	js/src/jsdbgapi.cpp:507
9 	xul.dll 	nsCOMPtr_base::assign_from_qi 	obj-firefox/xpcom/build/nsCOMPtr.cpp:96
10 	xul.dll 	nsScriptSecurityManager::GetSubjectPrincipal 	caps/src/nsScriptSecurityManager.cpp:1917
11 	xul.dll 	nsCOMPtr_base::~nsCOMPtr_base 	obj-firefox/dist/include/nsAutoPtr.h:908
12 	xul.dll 	nsScriptSecurityManager::GetSubjectPrincipal 	caps/src/nsScriptSecurityManager.cpp:1917
13 	xul.dll 	IsCallerSecure 	dom/src/storage/nsDOMStorage.cpp:169
14 	xul.dll 	nsDOMStorage::RemoveItem 	dom/src/storage/nsDOMStorage.cpp:1636
15 	xul.dll 	castNative 	js/xpconnect/src/XPCQuickStubs.cpp:767
16 	xul.dll 	js::GetPropertyHelper 	js/src/jsobj.cpp:5124
17 	xul.dll 	xpc_qsUnwrapThis<nsIDOMStorage> 	js/xpconnect/src/XPCQuickStubs.h:492
18 	xul.dll 	xpc_qsDOMString::xpc_qsDOMString 	js/xpconnect/src/XPCQuickStubs.cpp:605
19 	xul.dll 	nsDOMStorage2::RemoveItem 	dom/src/storage/nsDOMStorage.cpp:1961
20 	xul.dll 	nsIDOMStorage_RemoveItem 	obj-firefox/js/xpconnect/src/dom_quickstubs.cpp:18996
21 	xul.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:519
22 	xul.dll 	js::Interpret 	js/src/jsinterp.cpp:2757
23 	xul.dll 	nsDisplayList::HitTest 	layout/base/nsDisplayList.cpp:802
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsRefPtr%3Cmozilla%3A%3ADOMSVGAnimatedLengthList%3E%3A%3A~nsRefPtr%3Cmozilla%3A%3ADOMSVGAnimatedLengthList%3E%28%29+|+nsCOMPtr_base%3A%3A~nsCOMPtr_base%28%29+|+nsSessionStorageEntry%3A%3A~nsSessionStorageEntry%28%29

Comment 1

[Scoobidiver (away)]

I added the 32-bit signature.

It's now #1 top crasher over the last day with 60 crashes per hour!

Comment 3

[Honza Bambas (:mayhemer)]

Attachment: v1

Potential fix.  To explain:

  nsString oldValue;
  nsSessionStorageEntry *entry = mItems.GetEntry(aKey);         <<<<< here we store address of an entry in the hashtable

  if (entry && entry->mItem->IsSecure() && !aCallerSecure) {
    return NS_ERROR_DOM_SECURITY_ERR;
  }

  if (UseDB()) {
    nsresult rv = InitDB();
    NS_ENSURE_SUCCESS(rv, rv);

    CacheKeysFromDB();
    entry = mItems.GetEntry(aKey);                              <<<<< here we release all entries from the hashtable 
                                                                <<<<< (entry is no longer valid)

    nsAutoString value;
    bool secureItem;
    rv = GetDBValue(aKey, value, &secureItem);
    NS_ENSURE_SUCCESS(rv, rv);
    if (!aCallerSecure && secureItem)
      return NS_ERROR_DOM_SECURITY_ERR;

    oldValue = value;

    rv = gStorageDB->RemoveKey(this, aKey, !IsOfflineAllowed(mDomain),
                               aKey.Length() + value.Length());
    NS_ENSURE_SUCCESS(rv, rv);
  }
  else if (entry) {
    // clear string as StorageItems may be referencing this item
    oldValue = entry->mItem->GetValueInternal();
    entry->mItem->ClearValue();
  }

  if (entry) {
    mItems.RawRemoveEntry(entry);                               <<<<< here we delete it again (double delete)
  }
  aOldValue = oldValue;
  return NS_OK;


It's hard to locally reproduce, since often the entry is just created at the same address again, so no harm done, just by accident.
https://tbpl.mozilla.org/?tree=Try&rev=2d475a844444

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 618770 [details] [diff] [review]
v1

r=me

Comment 5

[Honza Bambas (:mayhemer)]

Comment on attachment 618770 [details] [diff] [review]
v1

https://hg.mozilla.org/mozilla-central/rev/0d6b3c17b839