CDB::Close should never be called with an activeTxn in progress.
Wallet: Replace TxnAbort with assert. #7071
pull pstratem wants to merge 1 commits into bitcoin:master from pstratem:2015-11-20-wallet-activetxn changing 1 files +3 −2-
pstratem commented at 9:15 PM on November 20, 2015: contributor
-
74aaae3b06
Wallet: Replace TxnAbort with assert.
CDB::Close should never be called with an activeTxn in progress.
-
petertodd commented at 9:29 PM on November 20, 2015: contributor
utACK
- jonasschnelli added the label Wallet on Nov 21, 2015
-
laanwj commented at 9:19 AM on November 24, 2015: member
I agree in principle, but assertions on shutdown are extremely annoying. People google the error that pops up.
They tend to mask the actual error that caused the (sudden) shutdown. E.g it will end up in this assertion if the node was aborted suddenly for some other reason, and it didn't manage to close the transaction. The original exception will be lost (I think).
We had a similar assert in
~LockedPageManager()for a long time - checking that the set of locked pages was empty on shutdown - and most bug report mentioned it in favor of more useful information from debug.log.As long as all the usages are in function scope this isn't a problem. CDB is not a global object, unlike the CDBEnv. But make sure that this doesn't become a problem.
-
pstratem commented at 9:39 AM on November 24, 2015: contributor
The only alternative I can think of would be to simply ignore the failure silently, however that could result in permanent data loss....
-
laanwj commented at 9:48 AM on November 24, 2015: member
The way in-between would be to log an error message and continue.
It is less in-your-face than an assertion, but then again, I'd expect when this happens there will already be something wrong and the user will be looking at / uploading debug.log.
Edit: and as I said this may not be an issue in this specific case at all. Just take it into account.
- pstratem closed this on Dec 13, 2015
- DrahtBot locked this on Sep 8, 2021
