fix crash on shutdown when e.g. changing -txindex and abort action #6282

1. 
   Diapolo commented at 5:49 am on June 15, 2015: none

   • fixes #3136
   • the problem is related to Boost path and a static initialized internal pointer
   • using a std::string in CDBEnv::EnvShutdown() prevents the problem
2. in src/wallet/db.h: in 38693906f8 outdated

   27@@ -28,6 +28,7 @@ class CDBEnv
   28     bool fDbEnvInit;
   29     bool fMockDb;
   30     boost::filesystem::path path;
   

   ---

   ---

   

   laanwj commented at 6:07 am on June 15, 2015:

   Shouldn’t this field go away, then?

   ---

   

   Diapolo commented at 6:17 am on June 15, 2015:

   I didn’t replace all occurances, because it’s used when building paths still.

   ---

   

   laanwj commented at 7:26 am on June 15, 2015:

   Do add a comment then, on why the duplicated field was added. Otherwise people will be really confused to read this code.
3. 
   laanwj commented at 7:31 am on June 15, 2015: member

   If boost::path gives any problems at static deinitialization, I still dont feel good with this.

   My suggestion would be to precompute all the usages of path in CDBEnv::Open. E.g.

   0std::string strPath;
   1std::string strPathLogDir;
   

   Then in the initialization

   0strPath = pathIn.string();
   1strPathLogDir = pathLogDir.string();
   

   Remove the boost::filesystem::path path field. Replace the usages of path outside CDBEnv::Open with these precomputed strings.

4. laanwj added the label Bug on Jun 15, 2015
5. 
   Diapolo commented at 11:44 am on June 15, 2015: none

   @laanwj That suggestion is fine, I tried to be minimal invasive but will gladly remove the path field.
6. 
   Diapolo commented at 12:36 pm on June 15, 2015: none

   @laanwj See pull after rebase, I removed the path field, but as quite some functions require a boost::filesystem::path I couldn’t eliminate all uses (e.g. TryCreateDirectory() and boost::filesystem::remove_all()). And also I didn’t want to play with the / operator used in boost::path as dir separator, so I didn’t replace it with the string /, which could cause problems on certain OSes, where a native path separator is perhaps different.
7. 
   Diapolo commented at 1:13 pm on June 15, 2015: none

   Seems the build faulire is unrelated to the pull: “No output has been received in the last 10 minutes, this potentially indicates a stalled build or something wrong with the build itself.

   The build has been terminated”

8. 
   laanwj commented at 1:41 pm on June 15, 2015: member

   Looks good to me, utACK, travis error is the misbehaving comparison tool again…
9. 
   Diapolo commented at 10:19 am on June 16, 2015: none

   Now it passed, great!
10. 
    laanwj commented at 12:09 pm on June 16, 2015: member

    I removed the path field, but as quite some functions require a boost::filesystem::path I couldn’t eliminate all uses

    Right. Eliminating the uses of boost::filesystem is not the goal here, just avoiding that a static filesystem::path field stays around as part of CDBEnv, which this does now.

11. fix crash on shutdown when e.g. changing -txindex and abort action

    - fixes #3136
    - the problem is related to Boost path and a static initialized internal
      pointer
    - using a std::string in CDBEnv::EnvShutdown() prevents the problem
    - this removes the boost::filesystem::path path field from CDBEnv

    0ce30eaa36
12. in src/wallet/db.h: in 0a821300e9 outdated

    26@@ -27,7 +27,7 @@ class CDBEnv
    27 private:
    28     bool fDbEnvInit;
    29     bool fMockDb;
    30-    boost::filesystem::path path;
    31+    std::string strPath;
    

    ---

    ---

    

    laanwj commented at 12:11 pm on June 16, 2015:

    Please add a comment here why this field is a string instead of a boost::filesystem::path, so that a future smart-ass doesn’t think “hey let’s use that type directly” and changes it back.

    ---

    

    Diapolo commented at 1:04 pm on June 16, 2015:

    Done!
13. 
    sipa commented at 1:43 pm on June 16, 2015: member

    utACK
14. laanwj merged this on Jun 18, 2015
15. laanwj closed this on Jun 18, 2015

    ---

16. laanwj referenced this in commit cbec57fd71 on Jun 18, 2015
17. Diapolo deleted the branch on Jun 19, 2015
18. laanwj referenced this in commit daf956b7b1 on Jun 23, 2015
19. 
    laanwj commented at 8:45 am on June 23, 2015: member

    Backported to 0.11 as daf956b7b196769f8027026dd047e72412a9a039, thanks @luke-jr for the heads up
20. MarcoFalke locked this on Sep 8, 2021

Contributors
Diapolo laanwj sipa

Labels
Bug