Closed Bug 957256 Opened 6 years ago Closed 6 years ago

crash in mozilla::scache::StartupCache::WriteToDisk()

Categories

(Core :: XPCOM, defect, critical)

x86
Windows NT
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla29
Tracking Status
firefox26 --- unaffected
firefox27 --- unaffected
firefox28 + verified
firefox29 --- verified

People

(Reporter: u279076, Assigned: aklotz)

References

Details

(Keywords: crash, topcrash-win)

Crash Data

Attachments

(1 file, 1 obsolete file)

This bug was filed from the Socorro interface and is 
report bp-7048eb14-841d-4957-ae50-a6a9d2140106.
=============================================================
0 	xul.dll 	mozilla::scache::StartupCache::WriteToDisk() 	startupcache/StartupCache.cpp
1 	nss3.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c
2 	nss3.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c
3 	msvcr100.dll 	_callthreadstartex 	f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c
4 	msvcr100.dll 	_threadstartex 	f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c
5 	kernel32.dll 	BaseThreadInitThunk 	
6 	ntdll.dll 	__RtlUserThreadStart 	
7 	ntdll.dll 	_RtlUserThreadStart

More reports: 
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3Ascache%3A%3AStartupCache%3A%3AWriteToDisk%28%29 	
=============================================================

This is currently a top-crash in Firefox Aurora 28 (#7 @ 1.76%) but is low volume in Firefox Nightly 29 (#65 @ 0.21%) and only a handful of crashes exist for Firefox Beta 27 and earlier. This is most highly correlated to Firefox Aurora 28 (87.19%) on Windows 7 (71.19%) on x86 (100%). It appears to have a crashes:install ration of 1.0 (177 crashes per 177 installs). 

It appears as though this crash has been around for some time but in relatively low volume. The first build ID to see a spike of >10 crashes was 2013123000 with 12 crashes and crashes have been at or above 15 per day since then.

Unfortunately there are no stand-out comments, URLs, or correlations to investigate.

Marked as security-sensitive since it's an EXCEPTION_ACCESS_VIOLATION_WRITE.
honza, michal, any idea what's going on here?
I'm pretty sure we're actually at http://hg.mozilla.org/releases/mozilla-aurora/annotate/809aabadac6d/startupcache/StartupCache.cpp#l439 and we're writing against a null gStartupCache pointer.
Group: core-security
Component: Networking: Cache → XPCOM
The main thread is at:

mozilla::scache::StartupCache::WaitOnWriteThread()
mozilla::scache::StartupCache::~StartupCache()
mozilla::scache::StartupCache::DeleteSingleton()
mozilla::ShutdownXPCOM(nsIServiceManager *)

The problem appears to be that DeleteSingleton nulls out gStartupCache but we don't wait on the write thread until *after* that. Those need to be reversed somehow.
Taras, this appears to be a result of your changes in bug 586859, although I'm not sure why we're hitting it more often now. Can you own/find an owner?
Flags: needinfo?(taras.mozilla)
Assignee: nobody → aklotz
Flags: needinfo?(taras.mozilla)
A pointer to the StartupCache object was being passed as context to the timer, however in both the timer callback and in the write thread we were referencing the object via gStartupCache.

This fix modifies those functions to use the aClosure parameter to obtain the pointer to the StartupCache object.
Attachment #8357977 - Flags: review?(benjamin)
As discussed on IRC, this is actually correct because the destructor cancels the timer and joins the write thread, but I'd really like a comment explaining why it's ok, in case we change something later.
Attachment #8357977 - Flags: review?(benjamin) → review+
Comments added as requested. Carrying forward r+.
Attachment #8357977 - Attachment is obsolete: true
Attachment #8358684 - Flags: review+
Comment on attachment 8358684 [details] [diff] [review]
Modify timer and thread callbacks to use context parameter to reference StartupCache object (v.2)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 586859
User impact if declined: Crashes due to null pointer dereference, particularly with Aurora 28.
Testing completed (on m-c, etc.): Locally
Risk to taking this patch (and alternatives if risky): None
String or IDL/UUID changes made by this patch: None
Attachment #8358684 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/3f8990e34520
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Duplicate of this bug: 957559
Attachment #8358684 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flagging for verification based on crashstats. We should probably give this at least a few days before checking.
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.