Bugfix: only track UTXO modification after lookup #5597

1. 
   sipa commented at 4:20 PM on January 4, 2015: member

   Otherwise, if CCoinsViewCache::ModifyCoins throws an exception in between setting hasModifier and constructing the CCoinsModifier, the cache ends up in an inconsistent state, resulting in an assert failure in the next modification.

   Bug discovered by @laanwj.

2. Bugfix: only track UTXO modification after lookup

   Otherwise, if CCoinsViewCache::ModifyCoins throws an exception in between
   setting hasModifier and constructing the CCoinsModifier, the cache ends up
   in an inconsistent state, resulting in an assert failure in the next
   modification.
   
   Bug discovered by Wladimir J. van der Laan.

   02bced1661
3. 
   laanwj commented at 4:38 PM on January 4, 2015: member

   Tested ACK (e.g. https://github.com/laanwj/bitcoin/tree/2014_12_leveldb_crash_fuzzing )

4. laanwj added this to the milestone 0.10.0 on Jan 4, 2015
5. Catch LevelDB errors during flush e41345790f
6. 
   sipa commented at 6:12 PM on January 4, 2015: member

   Added a commit to catch and shutdown gracefully in case of LevelDB errors during flush.

7. 
   sipa commented at 11:57 PM on January 5, 2015: member

   This is not sufficient (as testing by @gmaxwell shows), there are many cases where reads from the database can be triggered and aren't caught.

8. 
   laanwj commented at 9:21 AM on January 6, 2015: member

   Yes, leveldb exceptions an escape from CCoinsViewDB in many other places as just the flush, and if they happen in the network code they are caught in the top-level handler, be logged, and the process will happily continue.

   However, as this is strictly an improvement we should still merge this?

9. 
   sipa commented at 2:54 PM on January 6, 2015: member

   @laanwj Yes, I think it's definitely an improvement.

10. 
    gmaxwell commented at 6:02 PM on January 6, 2015: contributor

    Tested ACK. (As pieter notes, we still have things to fit in this respect, but better is better)

11. laanwj merged this on Jan 7, 2015
12. laanwj closed this on Jan 7, 2015

    ---

13. laanwj referenced this in commit 7625f7ff94 on Jan 7, 2015
14. sipa referenced this in commit 008138c04a on Jan 7, 2015
15. sipa referenced this in commit 867c600c29 on Jan 7, 2015
16. in src/main.cpp:None in e41345790f

    1806 | @@ -1807,6 +1807,7 @@ enum FlushStateMode {
    1807 |  bool static FlushStateToDisk(CValidationState &state, FlushStateMode mode) {
    1808 |      LOCK(cs_main);
    1809 |      static int64_t nLastWrite = 0;
    1810 | +    try {
    

    ---

    ---

    

    Diapolo commented at 2:09 PM on January 7, 2015:

    @sipa Why did you choose to leave out the indentation? Our code get's pretty ugly again over time? Could we finally let the clang-format tool let do it's job and we also had a remove orphan spaces and tabs script if I remember correctly...

17. reddink referenced this in commit 48bd2f230c on May 27, 2020
18. reddink referenced this in commit 6c470707ec on May 27, 2020
19. MarcoFalke locked this on Sep 8, 2021

Milestone
0.10.0