1159967 - crash in mozilla::dom::indexedDB::LoggingIdString::LoggingIdString()

Status: VERIFIED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Mats Palmgren (inactive)]

This bug was filed from the Socorro interface and is 
report bp-b322976e-7e40-464c-b4be-c5e472150405.
=============================================================

New signature in 38/39, first reported in build 2015033015.

Over the past 28 days:
Product	Version	Percentage	Number Of Crashes
Firefox 	38.0b1 	23.03 %	863
Firefox 	38.0b6 	19.69 %	738
Firefox 	38.0b3 	16.14 %	605
Firefox 	38.0b5 	15.26 %	572
Firefox 	38.0b2 	13.21 %	495
Firefox 	38.0b4 	11.07 %	415
Firefox 	39.0a2 	1.28 %	48
Firefox 	38.0a2 	0.16 %	6
Firefox 	38.0b8 	0.11 %	4
Firefox 	38.0b7 	0.05 %	2 

stack:
mozilla::dom::indexedDB::LoggingIdString::LoggingIdString()
mozilla::dom::indexedDB::IDBTransaction::OnRequestFinished(bool)
mozilla::dom::indexedDB::BackgroundRequestChild::ActorDestroy(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason)
mozilla::dom::cellbroadcast::PCellBroadcastParent::DestroySubtree(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason)
mozilla::dom::indexedDB::PBackgroundIDBTransactionChild::DestroySubtree(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason)
mozilla::dom::indexedDB::PBackgroundIDBDatabaseChild::DestroySubtree(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason)
mozilla::dom::indexedDB::PBackgroundIDBFactoryChild::DestroySubtree(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason)
mozilla::ipc::PBackgroundChild::DestroySubtree(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason)
mozilla::ipc::PBackgroundChild::OnChannelClose()
mozilla::ipc::MessageChannel::NotifyChannelClosed()
`anonymous namespace'::ChildImpl::ThreadLocalDestructor(void*)
PR_SetThreadPrivate
PR_GetThreadPrivate
`anonymous namespace'::ChildImpl::Shutdown()
`anonymous namespace'::ParentImpl::ShutdownObserver::Observe(nsISupports*, char const*, wchar_t const*)
nsObserverList::NotifyObservers(nsISupports*, char const*, wchar_t const*)
nsObserverService::NotifyObservers(nsISupports*, char const*, wchar_t const*)
ScopedXPCOMStartup::~ScopedXPCOMStartup()
ScopedXPCOMStartup::`scalar deleting destructor'(unsigned int)
XREMain::XRE_main(int, char** const, nsXREAppData const*)
XRE_main
do_main

Comment 1

[Mats Palmgren (inactive)]

[Tracking Requested - why for this release]:
It's #26 in Top Crashers for Firefox 38.0b.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Tracking as it is a top crash.
Jan, can you help with this? Thanks

Comment 3

[Jan Varga [:janv]]

Ben, I think ChildImpl::Shutdown() already cleared the thread private info and LoggingIdString::LoggingIdString() is trying to access it.

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

This patch null-checks appropriately, avoids doing anything if logging is disabled, and only converts the nsID -> nsCString once for child side logging.

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8599984 [details] [diff] [review]
Patch, v1

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

::: dom/indexedDB/ProfilerHelpers.h
@@ +19,5 @@
>  #include "IDBObjectStore.h"
>  #include "IDBTransaction.h"
>  #include "IndexedDatabaseManager.h"
>  #include "Key.h"
> +#include "mozilla/ArrayUtils.h"

I will remove this, it's not needed.

Comment 6

[Jan Varga [:janv]]

Comment on attachment 8599984 [details] [diff] [review]
Patch, v1

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

Looks good.

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/641a358d8f11

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/641a358d8f11

Comment 9

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8599984 [details] [diff] [review]
Patch, v1

Approval Request Comment
[Feature/regressing bug #]: bug 1092311
[User impact if declined]: Shutdown crash
[Describe test coverage new/current, TreeHerder]: Tests see this occasionally
[Risks and why]: This is a very safe patch, basically just a null check.
[String/UUID change made/needed]: None

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8599984 [details] [diff] [review]
Patch, v1

[Triage Comment]
As it is a top crash, I am taking it.

Comment 11

[Sylvestre Ledru [:Sylvestre]]

And as it is Sunday, I am going to start the build today, I don't want to bother Ryan and I want this patch in the RC, it uplifted the patch myself.

https://hg.mozilla.org/releases/mozilla-release/rev/257a4e9e8236

Comment 12

[Ryan VanderMeulen [:RyanVM]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #11)
> And as it is Sunday, I am going to start the build today, I don't want to
> bother Ryan and I want this patch in the RC, it uplifted the patch myself.

\m/

Comment 13

[Ryan VanderMeulen [:RyanVM]]

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

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr38/rev/257a4e9e8236

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/257a4e9e8236

Comment 16

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Socorro [1] shows no more crashes over the past 4 weeks, in builds after the patch landed. The latest crashes were in 39 builds after May 4th.

I'm marking this as verified.

[1] - https://crash-stats.mozilla.com/report/list?range_unit=days&range_value=28&signature=mozilla%3A%3Adom%3A%3AindexedDB%3A%3ALoggingIdString%3A%3ALoggingIdString%28%29