[Qt] Do you want to rebuild the block database now? No -> crash #3136

I'm using -txindex in my normal environment, when using payment requests without the client running, the client is started without any parameters and detects the missing -txindex and asks me Do you want to rebuild the block database now? answering no here crashes the client.

https://github.com/bitcoin/bitcoin/blob/master/src/init.cpp#L834 @sipa Can you check that part of the code, as most of it was done by you :).

Edit: Error is on Win7 x64 and it's a BEX (Buffer Overflow Exception).

I can't reproduce this (on linux). I've had a problem with my testnet wallet so needed to rebuild a few times yesterday. When I clicked "abort" there was no crash. I'm not using -txindex though.

Perhaps you can give it a try with -txindex then?

I tried to debug this, but failed badly... I get a segfault related to path.hpp, which is from Boost filesystem. This happens after 2013-11-14 20:09:45 Shutdown : done. I just see that the object contains the correct path to my testnet3 datadir and is unrelated to our datadir caching (as I disabled it for testing purposes).

Perhaps there is some global that get's destructed after everything is already shutdown, dunno yet... @laanwj Any idea how I can nail this down further?

Edit: I'm going to try Boost 1.55.0 tomorrow and see if that helps ;).

Yes that's probably an destruction order problem, do you get a traceback?

Otherwise I don't know a good way to debug it except for selectively commenting out code.

I wonder if this also happens with gitian builds.

Does this still happen with current master?

Tested a minute ago, yes still happens with 0.9.1.

<pre> Problemereignisname: BEX64 Anwendungsname: bitcoin-qt.exe Anwendungsversion: 0.9.1.0 Anwendungszeitstempel: 51a93980 Fehlermodulname: StackHash_e7de Fehlermodulversion: 0.0.0.0 Fehlermodulzeitstempel: 00000000 Ausnahmeoffset: PCH_AE_FROM_ntdll+0x000000000009B13A Ausnahmecode: c0000005 Ausnahmedaten: 0000000000000008 </pre>

I found out that this code in path.hpp causes the segfault:

<pre> # ifdef BOOST_WINDOWS_API const std::string string() const { return string(codecvt()); } </pre>

This line causes it: DbEnv(0).remove(path.string().c_str(), 0); https://github.com/bitcoin/bitcoin/blob/master/src/db.cpp#L49

I was able to prevent the crash by not using a boost::path there, but add a std::string to the class, init that with path.string() at the beginning and use that in the above call. @laanwj Any idea for this? A use-after-free or something like that?

This is yet another shutdown order issue.

CDbEnv ideally shouldn't be a global object that has the lifetime of the program. I remember this also came up for testing by @SergioDemianLerner #4688.

I tested this on Windows 8.1 x64 and Windows 10 TP with the 0.9, 0.10 releases, as well as the master branch (build via Gitian route).

To reproduce:

1. Start bitcoin-qt.exe, pointing to a new datadir (-datadir=xxx, -choosedatadir)
2. Shutdown the client
3. Start bitcoin-qt.exe -txindex=1
4. Decline to rebuild index
5. Crash

A similar event can be triggered and observed, when executing bitcoind.exe without rpcuser and rpcpassword defined, which also results in an application crash.

FWIW, I uploaded the crash report, including error dump files: http://bitwatch.co/uploads/AppCrash_bitcoin-qt_w10x64.zip (1.414 KB)

Running Process Monitor hints a buffer overflow during the shutdown:

This error does not occure, when using -disablewallet or a wallet-less build.

As follow up: it's a memory access violation and I sort-of debugged it on an assembler level, where I saw that the crash occured when jumping to 0xFEEEFEEE + x, which indicates previously freed memory on Windows.

The hint about const std::string string() const { return string(codecvt()); } was golden, as this indeed pointed to root cause. Boost path uses a static initialized pointer internally, which appears to be deinitialized during shutdown, but if this is not done by the main thread, the pointer appears to be already deinitialized, even though there is still an attempted access later. An appearingly working solution was to force the initialization by the main thread.

Tested with #5877 on top of current master (https://builds.jonasschnelli.ch/pulls/5877/) on Windows 7. Crash no longer happens. Testes also current master on same Window 7 instance and could reproduce the reindex->abort crash.

Thanks a lot for testing @jonaschnelli! Closing this.

@laanwj This needs to be reopened, as the bug was reintroduced or is at least still present! I'm going for a much simpler fix and will open a pull in a few minutes.

See #6282...