1471822 - Crash in sqlite3_extended_result_codes

Status: RESOLVED WORKSFORME

(Toolkit :: Places, defect, P3)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-272404fd-cf75-47e1-a220-ce8250180627.
=============================================================

Top 10 frames of crashing thread:

0 nss3.dll sqlite3_extended_result_codes db/sqlite3/src/sqlite3.c:150419
1 xul.dll mozilla::storage::Connection::prepareStatement storage/mozStorageConnection.cpp:1231
2 xul.dll mozilla::storage::Statement::initialize storage/mozStorageStatement.cpp:132
3 xul.dll mozilla::storage::Connection::CreateStatement storage/mozStorageConnection.cpp:1832
4 xul.dll static nsresult mozilla::places::`anonymous namespace'::MigrateV48FrecenciesRunnable::Run toolkit/components/places/Database.cpp:2440
5 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1051
6 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:519
7 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:334
8 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:318
9 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:298

=============================================================

crash reports with this signature are showing up in greater numbers than in the past at the start of the 62.0b cycle - probably due to the places migration in bug 1463017.

Comment 1

[Marco Bonardo [:mak]]

(In reply to [:philipp] from comment #0)
> crash reports with this signature are showing up in greater numbers than in
> the past at the start of the 62.0b cycle - probably due to the places
> migration in bug 1463017.

That migration problem (large wal) is fixed, so I don't think there's a relation with that specific bug.
Though, it starts from MigrateV48FrecenciesRunnable.
It's crashing on a call to sqlite3_mutex_enter(db->mutex);

It's a crash on shutdown, just check the Shutdown Progress aggregation, we're likely not handling properly the database closing under us.

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

In automation: https://treeherder.mozilla.org/logviewer.html#?job_id=185312509&repo=try&lineNumber=1622

Comment 3

[Mike Taylor [:miketaylr]]

Pretty low volume, let's fix-optional for 62 (tho we'd likely want a patch for uplift depending on complexity).

Comment 4

[Pascal Chevrel:pascalc]

Only 3 crashes on the 63 branch (nightly and beta) since July, marking as fix-optional for 63.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Since the crash volume is low (less than 5 per week), the severity is downgraded to S3. Feel free to change it back if you think the bug is still critical.

For more information, please visit auto_nag documentation.

Comment 6

[Marco Bonardo [:mak]]

There's very few crashes left.
All of them are on shutdown, and it looks like cases where we are executing an async task, but it's late on shutdown:

1. mozilla::storage::(anonymous namespace)::AsyncInitializeClone::Run()
2. mozilla::places::AsyncGetFaviconDataForPage::Run()

It's all cases where we should likely give up, since we're shutting down, but we're not handling that case correctly.
I imagine sqlite3_extended_result_codes is crashing because the sqlite3* is a dangling pointer.

I think we should wrap up the readonly cloned connections into a separate object, that handles shutdown internally, and then reuse this connection for both favicons and history links.

In addition at least AsyncInitializeClone::Run is missing some checks as it should not be try to use a dead sqlite3 pointer, so this check may be wrong, or better it's not holding a mutex to check connectionReady(), it should probably lock sharedAsyncExecutionMutex?
https://searchfox.org/mozilla-central/rev/754074e05178e017ef6c3d8e30428ffa8f1b794d/storage/mozStorageConnection.cpp#1623

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Closing because no crashes reported for 12 weeks.