889089 - crash in mozilla::places::History::FetchPageInfo

Status: VERIFIED FIXED

(Toolkit :: Places, defect)

Description

[Scoobidiver (away)]

It first showed up in 24.0a1/20130620 but is discontinuous across builds.

Based on the stack trace, it's likely a regression from bug 834539.

Notice that it still occurs after the fix of bug 885666, another regression from bug 834539.

Signature 	mozilla::places::History::FetchPageInfo(mozilla::places::VisitData&, bool*) More Reports Search
UUID 	81bacd2c-a51b-4d28-8fc4-41a1d2130629
Date Processed	2013-06-29 11:59:09.074662
Uptime	5
Last Crash	10 seconds before submission
Install Age 	37461 since version was first installed.
Install Time 	2013-06-29 01:34:38
Product 	Firefox
Version 	25.0a1
Build ID 	20130628031149
Release Channel 	nightly
OS 	Windows NT
OS Version 	6.1.7600
Build Architecture 	x86
Build Architecture Info 	GenuineIntel family 6 model 26 stepping 5 | None
Crash Reason 	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address 	0x0
App Notes 	
AdapterVendorID: 0x10de, AdapterDeviceID: 0x1080, AdapterSubsysID: 00000000, AdapterDriverVersion: 9.18.13.1422
D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ 

Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::places::History::FetchPageInfo(mozilla::places::VisitData &,bool *) 	toolkit/components/places/History.cpp
1 	xul.dll 	mozilla::places::`anonymous namespace'::InsertVisitedURIs::Run() 	toolkit/components/places/History.cpp
2 	xul.dll 	nsThread::ProcessNextEvent(bool,bool *) 	xpcom/threads/nsThread.cpp
3 	xul.dll 	nsThread::ThreadFunc(void *) 	xpcom/threads/nsThread.cpp
4 	nss3.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c
5 	nss3.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c
6 	msvcr100.dll 	_callthreadstartex 	f:\\dd\\vctools\\crt_bld\\self_x86\\crt\\src\\threadex.c
7 	msvcr100.dll 	_threadstartex 	f:\\dd\\vctools\\crt_bld\\self_x86\\crt\\src\\threadex.c
8 	kernel32.dll 	BaseThreadInitThunk 	
9 	ntdll.dll 	__RtlUserThreadStart 	
10 	ntdll.dll 	_RtlUserThreadStart

More reports at:
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3Aplaces%3A%3AHistory%3A%3AFetchPageInfo%28mozilla%3A%3Aplaces%3A%3AVisitData%26%2C+bool*%29

Comment 1

[Marco Bonardo [:mak]]

there's no error check on the stmt variable, in 2 points after the GetStatement calls. Should be a trivial fix.

Comment 2

[Asaf Romano (gone)]

Attachment: patch

Comment 3

[Marco Bonardo [:mak]]

Comment on attachment 770686 [details] [diff] [review]
patch

Review of attachment 770686 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/places/History.cpp
@@ +2140,5 @@
>        "SELECT guid, id, title, hidden, typed, frecency "
>        "FROM moz_places "
>        "WHERE url = :page_url "
>      );
> +    NS_ENSURE_TRUE(stmt, NS_ERROR_UNEXPECTED);

nit: may use NS_ENSURE_STATE(stmt);

@@ +2152,5 @@
>        "FROM moz_places "
>        "WHERE guid = :guid "
>      );
> +    NS_ENSURE_TRUE(stmt, NS_ERROR_UNEXPECTED);
> +

ditto

Comment 4

[Asaf Romano (gone)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3bfe035b2573

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3bfe035b2573

Comment 6

[Marco Bonardo [:mak]]

Comment on attachment 770686 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 834539
User impact if declined: crash
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low, null check
String or IDL/UUID changes made by this patch: none

Comment 7

[Marco Bonardo [:mak]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bf18c9cdbac1

Comment 8

[Manuela Muntean [Away]]

Verified as fixed, since there are only 3 crashes with 24.0b4 reported in Socorro, regarding last month.

Details here:

https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&reason_type=contains&date=2013-08-27&range_value=28&range_unit=days&hang_type=any&process_type=any&signature=mozilla%3A%3Aplaces%3A%3AHistory%3A%3AFetchPageInfo%28mozilla%3A%3Aplaces%3A%3AVisitData%26%2C+bool%2A%29

Comment 9

[u279076]

Verified fixed, no Firefox 25 crashes reported to Socorro with this signature.