Fix boost::interprocess::detail::winapi::get_last_bootup_time #986

1. 
   TheBlueMatt commented at 8:25 PM on March 25, 2012: member

   This fixes #981 and should fix #956

   I havent tested 956 after this patch, but it does fix #981 and afaict, they are actually the same underlying issue. It would be nice to have someone do more digging and find out what is actually going on in the win32 api calls boost is making and find the real issue, but this fixes the problem. Also, someone should file a boost bugreport.

2. Fix boost::interprocess::detail::winapi::get_last_bootup_time

   This fixes #981 and should fix #956

   1b327daeb5
3. 
   TheBlueMatt commented at 8:30 PM on March 25, 2012: member

   It seems to be a (somewhat hacky) workaround to https://svn.boost.org/trac/boost/ticket/5392

4. 
   TheBlueMatt commented at 8:32 PM on March 25, 2012: member

   Note that because /all/ of boost::interprocess is implemented in headers, this patch can be applied in contrib/gitian-descriptors/gitian-win32.yml or contrib/gitian-descriptors/boost-win32.yml I chose boost-win32.yml as it seemed more sane, but it does mean everyone has gets to rebuild boost

5. 
   Diapolo commented at 10:15 PM on March 25, 2012: none

   Prove me wrong, isn't boost::interprocess a compiled lib? It would make sense to get that bug fixed with the boost guys and use a safe work-around until this is done and we can upgrade.

6. 
   TheBlueMatt commented at 11:16 PM on March 25, 2012: member

   Though boost is a collection of compiled libs, as it turns out, boost::interprocess isnt. If you look in the boost source at boost/interprocess you will notice that there are no files that end in .cpp, all of them are hpp (header files). Nothing is compiled to a library in boost::interprocess.
   And, yes, it would be much better to have a nice workaround in bitcoin and let the boost guys fix the problem, however, afaict it would be impossible to do without modifying the boost source in some way (either removing the definitions of BOOST_INTERPROCESS_HAS_BOOTTIME, or this pull).

7. 
   Diapolo commented at 5:19 AM on March 26, 2012: none

   You are right, only true libs we have from boost are filesystem, program_options, system and thread.

8. 
   Diapolo commented at 5:40 AM on March 26, 2012: none

   I tested your fix and what it does (and what I should have observed before), is to create a directory in the boost_interprocess folder whose name is based on the Windows bootup time. This is broken in the official 1.47 and 1.49, too.

   I suggest we combine your work here and my work (with ipcRecover()), as there still could be a chance to have an orphan message queue file, but only during the current Windows session.

   Your fix ensures that after a restart a new folder with current bootup-time will be created and mine covers stale message queue files. Seems like a pretty decent combination :).

9. 
   TheBlueMatt commented at 5:48 AM on March 26, 2012: member

   Yea, I dont think the two are at all mutually exclusive. If boost's interprocess stuff were to die with bitcoin (not sure how boost handles it, did I read that it was thrown in a service somewhere or something?) without the OS dying, the same issue could still appear that ipcRecover fixes. That said, I dont think we need to merge ipcRecover in 0.6, its more of a safety net and being (hopefully) this late in the 0.6 release cycle, I dont think merging it would be the best choice.

10. 
    Diapolo commented at 6:06 AM on March 26, 2012: none

    I updated it a few minutes ago, but I have no problem with getting it integreated into early 0.7. It were many many hours of debugging and trial and error, so it would be very nice, to include it :).

11. 
    sipa commented at 1:53 PM on March 31, 2012: member

    The patch looks sane to me, the only thing we have to make sure is that it remains compatible with a range of boost releases (past and future...).

    Also, I don't really like how this is limited to gitian builds. Is it impossible to do from qmake?

12. 
    Diapolo commented at 12:23 PM on April 1, 2012: none

    It would make sens to get this integrated with boost, so we would not have to handle it in the future. Are we allowed to modify the boost headers, so we can redistribute the fixed version of that file in i.e. the deps.zip for Windows?

13. 
    Diapolo commented at 10:16 PM on April 2, 2012: none

    Update in the boost discussion: https://svn.boost.org/trac/boost/ticket/5392#comment:29 I will try the latest boost SVN version tomorrow / later.

14. 
    Diapolo commented at 9:03 PM on April 10, 2012: none

    See here what the boost dev had to say to your patch: https://svn.boost.org/trac/boost/ticket/5392#comment:29

    I did some further tests and it seems with boost 1.49 it's sufficient to edit a hpp as per https://svn.boost.org/trac/boost/ticket/5392#comment:29

15. 
    TheBlueMatt commented at 9:05 PM on April 10, 2012: member

    Yea, I never said it was suitable for upstream inclusion. It was written specifically to fix the issue in bitcoin.

16. 
    Diapolo commented at 9:10 PM on April 10, 2012: none

    No problem I only wanted to point out that we could achieve better IPC handling without a monkey-patch :). Your initial work was great and helped me a lot to progress further, so thanks!

17. TheBlueMatt closed this on Apr 10, 2012

    ---

18. 
    TheBlueMatt commented at 9:13 PM on April 10, 2012: member

    Alright then.

19. suprnurd referenced this in commit b40c3e5927 on Dec 5, 2017
20. lateminer referenced this in commit fdd0cdb72f on Oct 30, 2019
21. DrahtBot locked this on Sep 8, 2021

Milestone
0.7.0