Stop using boost::thread_group #17307

1. 
   laanwj commented at 8:22 pm on October 29, 2019: member

   boost::thread_group and its associated join/interrupt mechanism has no equivalent in any C++XX standard.

   The interrupt mechanism is basically a condition variable, per thread group, that interrupts boost::this_thread::sleep_for and boost::this_thread::sleep and raises boost::thread_interrupted in the member thread when group.interrupt() is called.

   Luckily, it’s only used for three things:

   • CScheduler
   • ThreadImport
   • Script checking threads

   Both of these could have their own interrupt mechanism. Handling “join” for a thread group is trivial.

   There’s also a bit of usage in the tests, but I think it’s related to the above main usages.

   It’s a blocker every time for changes such as #17253 and #16117.

2. laanwj added the label Utils/log/libs on Oct 29, 2019
3. fanquake added this to the "In progress" column in a project

   ---

4. 
   MarcoFalke commented at 1:13 am on October 30, 2019: member

   I believe this has been solved in #14464 and #14489. Those are up for grabs for someone to rebase and apply some nail polish to the diff :nail_care:
5. MarcoFalke added the label good first issue on Oct 30, 2019
6. 
   laanwj commented at 8:00 am on October 30, 2019: member

   I don’t think we need an approach like #14489 that replaces all sleep calls with interruptiblesleep, only the relevant ones in the mentioned cases.

   It looks like there is already a helper class for interruptible sleeps:

   0/*
   1    A helper class for interruptible sleeps. Calling operator() will interrupt
   2    any current sleep, and after that point operator bool() will return true
   3    until reset.
   4*/
   5class CThreadInterrupt
   

   So I think it’s a matter of using these in the right places?

7. 
   Patil2099 commented at 8:45 am on October 30, 2019: none

   @laanwj Can I work on this issue?
8. 
   laanwj commented at 9:06 am on October 30, 2019: member

   @laanwj Can I work on this issue?

   Sure!

9. 
   hebasto commented at 2:04 pm on October 30, 2019: member

   @MarcoFalke

   I believe this has been solved in #14464 and #14489. Those are up for grabs for someone to rebase and apply some nail polish to the diff

   #14489 has no label “Up for grabs”, though ;)

10. 
    adamjonas commented at 9:03 pm on October 31, 2019: member

    Note that I have a rebased #14464 branch, but haven’t opened a PR based on the close Boost → C++11 migration project for now topic of the dev chat today.
11. 
    laanwj commented at 11:24 am on November 1, 2019: member

    @adamjonas The point with closing the project was, from my side, to signal that all the low-hanging fruit is gone, or currently not worth picking (making our own versions of low-level string/date/time handling and such).

    I’m leaving this issue open because I think this structural work is still worth doing. I think, that besides removing the boost dependency, that using traditional threading primitives instead of “thread groups” and “interruptible threads” (“spooky action at a distance” through thread-local state) could improve the code and make it more understandable.

12. 
    MarcoFalke commented at 12:49 pm on November 1, 2019: member

    I believe removing boost header includes should continue for the following reasons:

    • std::thread in C++11 is mostly identical to std::thread in C++17. So when we reach C++17, we better have alreay dealt with removing boost::thread*, so that the only remaining things are fs and the optional/unique_ptr utils.
    • Apart from serialize.h, the boost headers are the most memory and cpu hungry to compile
    • Depending on less third party modules during the build is a “nice to have”
13. 
    laanwj commented at 2:20 pm on November 1, 2019: member

    Sure, I think “speed up the compile” (and reduce build-time memory requirements) might be an interesting project in itself. Removing unnecessary includes from include files is always a good thing, even without expicit ‘move away from boost’ rationale.
14. 
    MarcoFalke commented at 1:15 am on April 27, 2020: member

    Please review #18710
15. MarcoFalke closed this on Apr 27, 2020

    ---

16. 
    laanwj commented at 8:50 am on April 30, 2020: member

    I don’t think this should have been closed while boost::thread_group is still being used.
17. laanwj reopened this on Apr 30, 2020

    ---

18. laanwj referenced this in commit b386d37360 on Jan 25, 2021
19. sidhujag referenced this in commit af159bf7a7 on Jan 25, 2021
20. MarcoFalke removed the label good first issue on Jan 30, 2021
21. laanwj closed this on Feb 1, 2021

    ---

22. fanquake moved this from the "In progress" to the "Done" column in a project

    ---

23. DrahtBot locked this on Aug 18, 2022

Contributors
laanwj MarcoFalke Patil2099 hebasto adamjonas

Labels
Utils/log/libs