wallet: Reset BerkeleyDB handle after connection fails #13161

pull real-or-random wants to merge 2 commits into bitcoin:master from real-or-random:bdb-reset changing 1 files +6 −2
  1. real-or-random commented at 5:21 pm on May 3, 2018: member

    According to the BerkeleyDB docs, the DbEnv handle may not be accessed after close() has been called. This change ensures that we create a new handle after close() is called. This avoids a segfault when the first connection attempt fails and then a second connection attempt tries to call open() on the already closed DbEnv handle.

    Without the patch, bitcoindd reliably crashes in the second call to set_lg_dir() after close() if there is an issue with the database:

     02018-05-03T13:27:21Z Bitcoin Core version v0.16.99.0-a024a1841-dirty (debug build)
 1[...]
 22018-05-03T13:27:21Z Using wallet directory /home/tim/.bitcoin
 32018-05-03T13:27:21Z init message: Verifying wallet(s)...
 42018-05-03T13:27:21Z Using BerkeleyDB version Berkeley DB 4.8.30: (April  9, 2010)
 52018-05-03T13:27:21Z Using wallet wallet.dat
 62018-05-03T13:27:21Z BerkeleyEnvironment::Open: LogDir=/home/tim/.bitcoin/database 
 72018-05-03T13:27:21Z BerkeleyEnvironment::Open: Error -30974 opening database environment: DB_RUNRECOVERY: Fatal error, run database recovery
 82018-05-03T13:27:21Z Moved old /home/tim/.bitcoin/database to /home/tim/.bitcoin/database.1525354041.bak. Retrying.
 92018-05-03T13:27:21Z BerkeleyEnvironment::Open: LogDir=/home/tim/.bitcoin/database ErrorFile=/home/tim/.bitcoin/db.log
10[1]    14533 segmentation fault (core dumped)  ./src/bitcoind

    After the fix:

     02018-05-03T17:19:32Z Bitcoin Core version v0.16.99.0-cc09e3bd0-dirty (release build)
 1[...]
 22018-05-03T17:19:32Z Using wallet directory /home/tim/.bitcoin
 32018-05-03T17:19:32Z init message: Verifying wallet(s)...
 42018-05-03T17:19:32Z Using BerkeleyDB version Berkeley DB 4.8.30: (April  9, 2010)
 52018-05-03T17:19:32Z Using wallet wallet.dat
 62018-05-03T17:19:32Z BerkeleyEnvironment::Open: LogDir=/home/tim/.bitcoin/database ErrorFile=/home/tim/.bitcoin/db.log
 72018-05-03T17:19:32Z scheduler thread start
 82018-05-03T17:19:32Z BerkeleyEnvironment::Open: Error -30974 opening database environment: DB_RUNRECOVERY: Fatal error, run database recovery
 92018-05-03T17:19:32Z Moved old /home/tim/.bitcoin/database to /home/tim/.bitcoin/database.1525367972.bak. Retrying.
102018-05-03T17:19:32Z BerkeleyEnvironment::Open: LogDir=/home/tim/.bitcoin/database ErrorFile=/home/tim/.bitcoin/db.log
112018-05-03T17:19:32Z Cache configuration:
122018-05-03T17:19:32Z * Using 2.0MiB for block index database
132018-05-03T17:19:32Z * Using 8.0MiB for chain state database
142018-05-03T17:19:32Z * Using 440.0MiB for in-memory UTXO set (plus up to 286.1MiB of unused mempool space)
152018-05-03T17:19:32Z init message: Loading block index..
16[...]
  2. wallet: Reset BerkeleyDB handle after connection fails 
    According to the BerkeleyDB docs, the DbEnv handle may not be accessed
after close() has been called. This change ensures that we create a new
handle after close() is called. This avoids a segfault when the first
connection attempt fails and then a second connection attempt tries to
call open() on the already closed DbEnv handle.
    264c643809
  3. jamesob commented at 6:38 pm on May 3, 2018: member

    utACK https://github.com/bitcoin/bitcoin/pull/13161/commits/264c64380978d7b0bd3ecb56a861eb004cc58b01

    Nice first contribution.

  4. laanwj added the label Wallet on May 3, 2018
  5. fanquake commented at 3:30 am on May 4, 2018: member
    Concept ACK. @real-or-random What are you doing to simulate an issue with the database?
  6. real-or-random commented at 10:43 am on May 5, 2018: member

    Oh it was not simulated, the error was real. The log file says

    0Unacceptable log file /home/tim/.bitcoin/database/log.0000000001: unsupported log version 19
1Invalid log file: log.0000000001: Invalid argument
2PANIC: Invalid argument
3process-private: unable to find environment

    (I guess the last line appeared only later after bitcoind moved away the database directory.) Any idea what could be the reason for this? (I’m not claiming that this was due to normal usage, I was playing around.)

    For testing, I can reliable reproduce the database issue by moving the old database directory back in place.

  7. real-or-random commented at 12:38 pm on May 7, 2018: member
    Not really related but I think I know what caused the database error: The wallet was initially created with the official Arch Linux version, which is configured --with-incompatible-bdb (I didn’t know that), and then I tried to open the wallet with a version built against bdb 4.8.
  8. ryanofsky commented at 3:18 pm on May 8, 2018: member

    Would a simpler fix just be to drop the dbenv->close(0); line? Adding more initialization steps and resetting fDbEnvInit and fMockDb members would seem to make the retry code a little more convoluted than it needs to be. It also doesn’t seem good that the close return value isn’t checked and nothing is logged if it failed (also true before this PR).

    I also wonder if we could add a unit test or functional test to exercise this code. One way could be to create a dummy pathIn / "database" file and checking to make sure it gets renamed to database.{time}.bak and the wallet still gets loaded successfully.

  9. real-or-random commented at 11:36 am on May 9, 2018: member

    The documentation says we need the close():

    If DbEnv::open() fails, the DbEnv::close() method must be called to discard the DbEnv handle. (see https://docs.oracle.com/cd/E17275_01/html/api_reference/CXX/envopen.html)

    I’ve added proper logging if the close() fails.

    I’m not sure if a test is that helpful but I can certainly add one if people think it’s a good idea.

  10. wallet: Improve logging when BerkeleyDB environment fails to close b6f0b4d859
  11. real-or-random force-pushed on May 9, 2018
  12. ryanofsky commented at 5:05 pm on May 9, 2018: member
    utACK b6f0b4d8595df0241a6499cd57e7ddac56558bf1. Thanks for the fix!
  13. sipa commented at 5:20 pm on May 9, 2018: member
    Concept ACK
  14. laanwj commented at 1:06 pm on May 14, 2018: member
    utACK b6f0b4d8595df0241a6499cd57e7ddac56558bf1
  15. laanwj merged this on May 14, 2018
  16. laanwj closed this on May 14, 2018
  17. laanwj referenced this in commit 7cc1bd3aae on May 14, 2018
  18. hbxia commented at 3:53 am on May 25, 2018: none

    Hi Tim, I still meet the similar issue with the latest code (already has your fix):

    2018-05-25T02:14:01Z HTTP: creating work queue of depth 16 2018-05-25T02:14:01Z No rpcpassword set - using random cookie authentication 2018-05-25T02:14:01Z Generated RPC authentication cookie /root/.bitcoin/.cookie 2018-05-25T02:14:01Z HTTP: starting 4 worker threads 2018-05-25T02:14:01Z Using wallet directory /root/.bitcoin/wallets 2018-05-25T02:14:01Z init message: Verifying wallet(s)… 2018-05-25T02:14:01Z Using BerkeleyDB version Berkeley DB 4.8.30: (April 9, 2010) 2018-05-25T02:14:01Z Using wallet wallet.dat 2018-05-25T02:14:01Z BerkeleyEnvironment::Open: LogDir=/root/.bitcoin/wallets/database ErrorFile=/root/.bitcoin/wallets/db.log Segmentation fault

    My server info: Linux iZbp18y166co7d3pw5qf79Z 4.4.0-98-generic #121-Ubuntu SMP Tue Oct 10 14:24:03 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

    BTW, official downloaded BIN bitcoind has no issue on my server, but manually build bitcoind.

    Do you have any idea? or any build parameter needed?

    Thanks

  19. real-or-random commented at 7:27 am on May 25, 2018: member

    Hm, your problem should not be related to this pull request.

    These are the build instructions: https://github.com/bitcoin/bitcoin/blob/master/doc/build-unix.md#linux-distribution-specific-instructions If you still encounter the problem, I’d recommend you to open a new issue and paste the contents of the ErrorFile as mentioned in the log.

  20. hbxia commented at 8:33 am on May 25, 2018: none
    Tim, thanks for your info. My current work around is to disable wallet “./configure –disable-wallet”, because I do not need it so far.
  21. jasonbcox referenced this in commit 8915b2f748 on Oct 25, 2019
  22. PastaPastaPasta referenced this in commit 3ffc494bd4 on Jun 17, 2020
  23. PastaPastaPasta referenced this in commit 9e7eaea7a1 on Jun 27, 2020
  24. PastaPastaPasta referenced this in commit d9a899e6ae on Jun 28, 2020
  25. PastaPastaPasta referenced this in commit 5b873ebb69 on Jun 29, 2020
  26. MarcoFalke locked this on Sep 8, 2021


real-or-random jamesob fanquake ryanofsky sipa laanwj hbxia

Labels
Wallet

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-01 10:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me