0.16 Shutdown assertion

theuni commented at 8:50 pm on February 2, 2018: member

I always do a quick round of sanity checks before pushing code signatures. Those checks basically only consist of running the win/osx binaries, making sure the sigs are valid, and making sure that they display the correct version number. So basically, run, then kill after ~3 seconds.

This time around, the windows build crashed with an assertion. Since I have no tools in this VM, and because it’s a release binary, I can only provide screen-shot level debugging :(

bitcoin-0 16-assertion

theuni added this to the milestone 0.16.0 on Feb 2, 2018

TheBlueMatt commented at 9:30 pm on February 2, 2018: member

Looks like qt and bitcoind have different init handling (still, ugh) - bitcoind always calls Shutdown() if AppInitMain is called, even when it fails, but qt does not. Several things make the Shutdown() assumption, so it seems like that assert was well-placed to catch a bug…sadly. I dont know our qt stuff well enough to suggest a fix, however.

MarcoFalke added the label GUI on Feb 2, 2018

theuni commented at 11:03 pm on February 2, 2018: member

@TheBlueMatt Agreed about aligning bitcoind and bitcoin-qt if possible, rather than throwing in more app-specific behavior.

I have no experience with qt dev, but that won’t stop me from speculating wildly! From IRC discussion:

[13:15:15] <cfields> BlueMatt: i think i might see the issue [13:16:58] <cfields> BlueMatt: BitcoinApplication connects requestedShutdown to shutdown(), but SplashScreen connects it to close() [13:17:32] <cfields> so it looks like we should re-route that after AppInitMain has started

theuni commented at 10:12 pm on February 4, 2018: member

I suspected the assertion might’ve been caused by exiting during a utxo db upgrade. Trying to reproduce in similar conditions, I managed to hit a different assertion on macOS :(

Run bitcoin-qt -reindex-chainstate, click to exit as soon as possible

 0Assertion failed: (!hashBlock.IsNull()), function BatchWrite, file txdb.cpp, line 90.
 1* thread [#8](/bitcoin-bitcoin/8/): tid = 0x1daa31, 0x00007fff93155f06 libsystem_kernel.dylib`__pthread_kill + 10, name = 'bitcoin-shutoff', stop reason = signal SIGABRT
 2  frame [#0](/bitcoin-bitcoin/0/): 0x00007fff93155f06 libsystem_kernel.dylib`__pthread_kill + 10
 3    frame [#1](/bitcoin-bitcoin/1/): 0x00007fff972154ec libsystem_pthread.dylib`pthread_kill + 90
 4    frame [#2](/bitcoin-bitcoin/2/): 0x00007fff8c65c6df libsystem_c.dylib`abort + 129
 5    frame [#3](/bitcoin-bitcoin/3/): 0x00007fff8c623dd8 libsystem_c.dylib`__assert_rtn + 321
 6    frame [#4](/bitcoin-bitcoin/4/): 0x00000001002b5c57 bitcoin-qt`CCoinsViewDB::BatchWrite(this=<unavailable>, mapCoins=size=0, hashBlock=<unavailable>) + 4039 at txdb.cpp:90
 7    frame [#5](/bitcoin-bitcoin/5/): 0x0000000100437bf4 bitcoin-qt`CCoinsViewCache::Flush(this=0x000000011e150640) + 52 at coins.cpp:204
 8    frame [#6](/bitcoin-bitcoin/6/): 0x00000001002e2c4b bitcoin-qt`FlushStateToDisk(chainparams=<unavailable>, state=0x000070000029f280, mode=<unavailable>, nManualPruneHeight=<unavailable>) + 1403 at validation.cpp:2104
 9    frame [#7](/bitcoin-bitcoin/7/): 0x00000001002e2698 bitcoin-qt`FlushStateToDisk() + 56 at validation.cpp:2123
10    frame [#8](/bitcoin-bitcoin/8/): 0x0000000100168857 bitcoin-qt`Shutdown() + 1143 at init.cpp:228
11    frame [#9](/bitcoin-bitcoin/9/): 0x000000010000845d bitcoin-qt`BitcoinCore::shutdown(this=0x000000010ac55530) + 349 at bitcoin.cpp:315
12    frame [#10](/bitcoin-bitcoin/10/): 0x000000010212c18c QtCore`QObject::event(QEvent*) + 156
13    frame [#11](/bitcoin-bitcoin/11/): 0x00000001012ad97b QtWidgets`QApplicationPrivate::notify_helper(QObject*, QEvent*) + 251
14    frame [#12](/bitcoin-bitcoin/12/): 0x00000001012b02de QtWidgets`QApplication::notify(QObject*, QEvent*) + 5630
15    frame [#13](/bitcoin-bitcoin/13/): 0x0000000102104212 QtCore`QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) + 1058
16    frame [#14](/bitcoin-bitcoin/14/): 0x00000001021537db QtCore`QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) + 59
17    frame [#15](/bitcoin-bitcoin/15/): 0x0000000102100c1c QtCore`QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) + 412
18    frame [#16](/bitcoin-bitcoin/16/): 0x0000000101f5607e QtCore`QThread::exec() + 110
19    frame [#17](/bitcoin-bitcoin/17/): 0x0000000101f59c02 QtCore`QThreadPrivate::start(void*) + 338
20    frame [#18](/bitcoin-bitcoin/18/): 0x00007fff9721299d libsystem_pthread.dylib`_pthread_body + 131
21    frame [#19](/bitcoin-bitcoin/19/): 0x00007fff9721291a libsystem_pthread.dylib`_pthread_start + 168
22    frame [#20](/bitcoin-bitcoin/20/): 0x00007fff97210351 libsystem_pthread.dylib`thread_start + 13

theuni commented at 2:28 am on February 6, 2018: member

Ok, I think I see what’s going on.

Closing the window kicks off the shutdown process.
CCoinsViewDB::Upgrade() returns false if ShutdownRequested(), which is now the case.
That causes AppInitMain() to return false
The shutdown process is still going

From what I can tell, that means that there’s a race between the quit() signaled by stopThread, and the one in the failure case of initializeResult. Maybe it’s possible for requestedInitialize to be emitted before coreThread is even started?

I’m really out of my depth with qt. @jonasschnelli halp plz :)

laanwj removed this from the milestone 0.16.0 on Feb 6, 2018

laanwj added this to the milestone 0.16.1 on Feb 6, 2018

laanwj commented at 10:21 am on February 6, 2018: member

@theuni Good catch, thanks for testing!

That said - I’m moving this to the 0.16.1 milestone as it is rare enough, and only an assertion error at shutdown. We could include the fix if we need an rc2, but this shouldn’t be a blocker.

jonasschnelli commented at 6:18 am on February 7, 2018: contributor

Agree with @laanwj. I’ll try to find a solution for this…

fanquake commented at 2:17 pm on May 28, 2018: member

I’ve just finished a depends build of 0.16.1 (224a41d) on a Windows 10 VM. Seeing the same error. I used WSL 18.04.

MarcoFalke removed this from the milestone 0.16.1 on May 29, 2018

MarcoFalke added this to the milestone 0.17.0 on May 29, 2018

laanwj commented at 5:42 pm on July 30, 2018: member

We should check if this is still the case for master

MarcoFalke removed this from the milestone 0.17.0 on Aug 5, 2018

MarcoFalke added this to the milestone 0.18.0 on Aug 5, 2018

fanquake commented at 1:11 pm on February 6, 2019: member

Just ran a depends build of master (5029e94f855c73bad2e0ea6dc0378f33787ae131) on a Windows 10 VM (WSL 18.04.1 LTS). I’m no longer seeing the crash at shutdown behaviour.

MarcoFalke commented at 3:27 pm on February 6, 2019: member

Thanks for testing. I think it was a racy crash (intermittent), but if there are no new reports, this might have been fixed in the meantime.

MarcoFalke closed this on Feb 6, 2019

fanquake deleted a comment on Feb 6, 2019

DrahtBot locked this on Dec 16, 2021

0.16 Shutdown assertion #12337