From e260ea1dd41aeb02397a49e314a2a2dbcdf5cc89 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Tue, 18 Jun 2019 15:06:23 +0200 Subject: [PATCH 1/6] C++11: remove the need for Boost.Thread, even with CGAL_Core --- CGAL_Core/include/CGAL/CORE/MemoryPool.h | 35 ++---------------- .../doc/Documentation/Installation.txt | 20 ++--------- Installation/INSTALL.md | 1 - .../cmake/modules/CGALConfig_binary.cmake.in | 9 ----- .../cmake/modules/CGALConfig_install.cmake.in | 9 ----- Installation/cmake/modules/CGAL_Macros.cmake | 7 ---- .../cmake/modules/CGAL_SetupBoost.cmake | 36 +------------------ .../CGAL_SetupCGAL_CoreDependencies.cmake | 24 ------------- 8 files changed, 6 insertions(+), 135 deletions(-) diff --git a/CGAL_Core/include/CGAL/CORE/MemoryPool.h b/CGAL_Core/include/CGAL/CORE/MemoryPool.h index 0f015768f99..64121773f97 100644 --- a/CGAL_Core/include/CGAL/CORE/MemoryPool.h +++ b/CGAL_Core/include/CGAL/CORE/MemoryPool.h @@ -38,13 +38,6 @@ #include #include #include -#if CGAL_STATIC_THREAD_LOCAL_USE_BOOST || (defined(CGAL_HAS_THREADS) && BOOST_GCC) -// Force the use of Boost.Thread with g++ and C++11, because of the PR66944 -// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66944 -// See also CGAL PR #1888 -// https://github.com/CGAL/cgal/pull/1888#issuecomment-278284232 -# include -#endif #include // for placement new #include @@ -76,7 +69,7 @@ public: t = t->next; } //); - //CGAL_warning_msg(count == nObjects * blocks.size(), + //Cgal_warning_msg(count == nObjects * blocks.size(), // "Cannot delete memory as there are cyclic references"); if(count == nObjects * blocks.size()){ @@ -92,38 +85,16 @@ public: // Access the corresponding static global allocator. static MemoryPool& global_allocator() { -#if CGAL_STATIC_THREAD_LOCAL_USE_BOOST || (defined(CGAL_HAS_THREADS) && BOOST_GCC) - if(memPool_ptr.get() == nullptr) {memPool_ptr.reset(new Self());} - Self& memPool = * memPool_ptr.get(); -#endif + CGAL_STATIC_THREAD_LOCAL_VARIABLE_0(Self, memPool); return memPool; } private: - Thunk* head; // next available block in the pool + Thunk* head; // next available block in the pool std::vector blocks; -#if CGAL_STATIC_THREAD_LOCAL_USE_BOOST || (defined(CGAL_HAS_THREADS) && BOOST_GCC) - static boost::thread_specific_ptr memPool_ptr; -#elif defined(CGAL_HAS_THREADS) // use the C++11 implementation - static thread_local Self memPool; -#else // not CGAL_HAS_THREADS - static Self memPool; -#endif // not CGAL_HAS_THREADS }; -#if CGAL_STATIC_THREAD_LOCAL_USE_BOOST || (defined(CGAL_HAS_THREADS) && BOOST_GCC) -template -boost::thread_specific_ptr > -MemoryPool::memPool_ptr; -#else // use C++11 or without CGAL_HAS_THREADS -template -# ifdef CGAL_HAS_THREADS -thread_local -# endif -MemoryPool MemoryPool::memPool; -#endif - template< class T, int nObjects > void* MemoryPool< T, nObjects >::allocate(std::size_t) { if ( head == 0 ) { // if no more memory in the pool diff --git a/Documentation/doc/Documentation/Installation.txt b/Documentation/doc/Documentation/Installation.txt index e4615d684fb..e8754dbcdb4 100644 --- a/Documentation/doc/Documentation/Installation.txt +++ b/Documentation/doc/Documentation/Installation.txt @@ -295,7 +295,7 @@ We next list the libraries and essential 3rd party software | Library | CMake Variable | Functionality | Dependencies | | :-------- | :------------- | :------------ | :----------- | -| `%CGAL` | none | Main library | \sc{Gmp}, \sc{Mpfr}, \sc{Boost} (headers), Boost.Thread and Boost.System (library) for compilers not supporting the keywords `thread_local` and the class `std::mutex` | +| `%CGAL` | none | Main library | \sc{Gmp}, \sc{Mpfr}, \sc{Boost} (headers) | | `CGAL_Core` | `WITH_CGAL_Core` | The CORE library for algebraic numbers.\cgalFootnote{CGAL_Core is not part of \cgal, but a custom version of the \sc{Core} library distributed by \cgal for the user convenience and it has it's own license.} | \sc{Gmp} and \sc{Mpfr} | | `CGAL_ImageIO` | `WITH_CGAL_ImageIO` | Utilities to read and write image files | \sc{zlib}, \sc{Vtk}(optional) | | `CGAL_Qt5` | `WITH_CGAL_Qt5` | `QGraphicsView` support for \sc{Qt}5-based demos | \sc{Qt}5 | @@ -388,27 +388,11 @@ The \sc{Boost} libraries are a set of portable C++ source libraries. Most of \sc{Boost} libraries are header-only, but a few of them need to be compiled or installed as binaries. -\cgal requires the \sc{Boost} libraries. In particular the header files -and the threading library (`Boost.Thread` and -`Boost.System` binaries). Version 1.48 (or higher) are needed -for compilers not supporting the keyword `thread_local` and the class `std::mutex` (This is supported for \gcc 4.8 and later when using the \cpp 11 standard, and for Visual C++ starting with 2015, that is VC14). - -As an exception, because of a bug in the \gcc compiler about the \cpp 11 -keyword `thread_local`, the `CGAL_Core` library always requires -`Boost.Thread` if the \gcc compiler is used. - -On Windows, as auto-linking is used, you also need the binaries of -`Boost.Serialization` and `Boost.DateTime`, but the -dependency is artificial and used only at link-time: the \cgal libraries do -not depend on the DLL's of those two libraries. - -In \cgal some demos and examples depend on `Boost.Program_options`. +\cgal only requires the headers of the \sc{Boost} libraries, but some demos and examples depend on the binary library `Boost.Program_options`. In case the \sc{Boost} libraries are not installed on your system already, you can obtain them from `https://www.boost.org/`. For Visual C++ you can download precompiled libraries from `https://sourceforge.net/projects/boost/files/boost-binaries/`. -For Visual C++ versions prior to 2015 `Boost.Thread` is required, so make sure to either install the precompiled -libraries for your compiler or build `libboost-thread` and `libboost-system`. As on Windows there is no canonical directory for where to find \sc{Boost}, we recommend that you define the environment variable diff --git a/Installation/INSTALL.md b/Installation/INSTALL.md index c14c6ec5493..eeefd10cf37 100644 --- a/Installation/INSTALL.md +++ b/Installation/INSTALL.md @@ -31,7 +31,6 @@ CGAL packages, some are only needed for demos. * Boost (>= 1.48) Required for building CGAL and for applications using CGAL - Required compiled Boost library: Boost.Thread, Boost.System Optional compiled Boost library: Boost.Program_options http://www.boost.org/ or http://www.boostpro.com/products/free/ You need the former if you plan to compile the boost libraries yourself, diff --git a/Installation/cmake/modules/CGALConfig_binary.cmake.in b/Installation/cmake/modules/CGALConfig_binary.cmake.in index 23a6399923e..abc58be24ed 100644 --- a/Installation/cmake/modules/CGALConfig_binary.cmake.in +++ b/Installation/cmake/modules/CGALConfig_binary.cmake.in @@ -114,15 +114,6 @@ macro(check_cgal_component COMPONENT) if ( "${CGAL_LIB}" STREQUAL "CGAL" ) set( CGAL_FOUND TRUE ) set( CHECK_CGAL_ERROR_TAIL "" ) - get_property(CGAL_CGAL_is_imported TARGET CGAL::CGAL PROPERTY IMPORTED) - if(CGAL_CGAL_is_imported) - include("${CGAL_MODULES_DIR}/CGAL_SetupBoost.cmake") - get_property(CGAL_requires_Boost_libs - GLOBAL PROPERTY CGAL_requires_Boost_Thread) - if(CGAL_requires_Boost_libs AND TARGET Boost::thread) - set_property(TARGET CGAL::CGAL APPEND PROPERTY INTERFACE_LINK_LIBRARIES Boost::thread) - endif() - endif() else( "${CGAL_LIB}" STREQUAL "CGAL" ) if ( WITH_${CGAL_LIB} ) if(TARGET CGAL::${CGAL_LIB}) diff --git a/Installation/cmake/modules/CGALConfig_install.cmake.in b/Installation/cmake/modules/CGALConfig_install.cmake.in index a97e23a1a8b..17606ef1ddd 100644 --- a/Installation/cmake/modules/CGALConfig_install.cmake.in +++ b/Installation/cmake/modules/CGALConfig_install.cmake.in @@ -82,15 +82,6 @@ macro(check_cgal_component COMPONENT) # include config file include(${CGAL_CONFIG_DIR}/CGALLibConfig.cmake) set( CHECK_CGAL_ERROR_TAIL "" ) - get_property(CGAL_CGAL_is_imported TARGET CGAL::CGAL PROPERTY IMPORTED) - if(CGAL_CGAL_is_imported) - include("${CGAL_MODULES_DIR}/CGAL_SetupBoost.cmake") - get_property(CGAL_requires_Boost_libs - GLOBAL PROPERTY CGAL_requires_Boost_Thread) - if(CGAL_requires_Boost_libs AND TARGET Boost::thread) - set_property(TARGET CGAL::CGAL APPEND PROPERTY INTERFACE_LINK_LIBRARIES Boost::thread) - endif() - endif() else() if (EXISTS ${CGAL_CONFIG_DIR}/${CGAL_LIB}Exports.cmake) # include export files for requested component diff --git a/Installation/cmake/modules/CGAL_Macros.cmake b/Installation/cmake/modules/CGAL_Macros.cmake index 23049fbb26e..4d501fa5ce5 100644 --- a/Installation/cmake/modules/CGAL_Macros.cmake +++ b/Installation/cmake/modules/CGAL_Macros.cmake @@ -290,13 +290,6 @@ if( NOT CGAL_MACROS_FILE_INCLUDED ) # To deal with imported targets of Qt5 and Boost, when CGAL # targets are themselves imported by another project. if(NOT CGAL_BUILDING_LIBS) - if (NOT MSVC AND "${component}" STREQUAL "Core") - # See the release notes of CGAL-4.10: CGAL_Core now requires - # Boost.Thread, with all compilers but MSVC. - find_package( Boost 1.48 REQUIRED thread system ) - add_to_list( CGAL_3RD_PARTY_LIBRARIES ${Boost_LIBRARIES} ) - endif() - if (${component} STREQUAL "Qt5") find_package(Qt5 COMPONENTS OpenGL Gui Core Script ScriptTools QUIET) endif() diff --git a/Installation/cmake/modules/CGAL_SetupBoost.cmake b/Installation/cmake/modules/CGAL_SetupBoost.cmake index 758736e3dd0..a1b57c24423 100644 --- a/Installation/cmake/modules/CGAL_SetupBoost.cmake +++ b/Installation/cmake/modules/CGAL_SetupBoost.cmake @@ -17,38 +17,7 @@ set ( CGAL_Boost_Setup TRUE ) include(${CMAKE_CURRENT_LIST_DIR}/CGAL_TweakFindBoost.cmake) -function(CGAL_detect_if_Boost_Thread_is_required) - get_property(PROPERTY_CGAL_requires_Boost_Thread_IS_SET - GLOBAL PROPERTY CGAL_requires_Boost_Thread SET) - if(PROPERTY_CGAL_requires_Boost_Thread_IS_SET) - get_property(CGAL_requires_Boost_libs - GLOBAL PROPERTY CGAL_requires_Boost_Thread) - else() - set ( CGAL_requires_Boost_libs TRUE ) - if ( DEFINED MSVC_VERSION AND "${MSVC_VERSION}" GREATER 1800) - set ( CGAL_requires_Boost_libs FALSE ) - else() - try_run( CGAL_test_cpp_version_RUN_RES CGAL_test_cpp_version_COMPILE_RES - "${CMAKE_BINARY_DIR}" - "${CGAL_MODULES_DIR}/config/support/CGAL_test_cpp_version.cpp" - RUN_OUTPUT_VARIABLE CGAL_cplusplus) - message(STATUS "__cplusplus is ${CGAL_cplusplus}") - if(NOT CGAL_test_cpp_version_RUN_RES) - set ( CGAL_requires_Boost_libs FALSE ) - message(STATUS " --> Do not link with Boost.Thread") - endif() - endif() - endif() - set_property(GLOBAL PROPERTY CGAL_requires_Boost_Thread ${CGAL_requires_Boost_libs}) - set(CGAL_requires_Boost_libs ${CGAL_requires_Boost_libs} PARENT_SCOPE) -endfunction(CGAL_detect_if_Boost_Thread_is_required) - -CGAL_detect_if_Boost_Thread_is_required() -if (CGAL_requires_Boost_libs) - find_package( Boost 1.48 REQUIRED thread system ) -else() - find_package( Boost 1.48 REQUIRED ) -endif() +find_package( Boost 1.48 REQUIRED ) if(Boost_FOUND AND Boost_VERSION VERSION_LESS 1.70) if(DEFINED Boost_DIR AND NOT Boost_DIR) @@ -94,9 +63,6 @@ function(use_CGAL_Boost_support target) endif() if(TARGET Boost::boost) target_link_libraries(${target} ${keyword} Boost::boost) - if (CGAL_requires_Boost_libs) - target_link_libraries(${target} ${keyword} Boost::thread) - endif() else() target_include_directories(${target} SYSTEM ${keyword} ${Boost_INCLUDE_DIRS}) target_link_libraries(${target} ${keyword} ${Boost_LIBRARIES}) diff --git a/Installation/cmake/modules/CGAL_SetupCGAL_CoreDependencies.cmake b/Installation/cmake/modules/CGAL_SetupCGAL_CoreDependencies.cmake index 6c0f2ad214d..609f086694b 100644 --- a/Installation/cmake/modules/CGAL_SetupCGAL_CoreDependencies.cmake +++ b/Installation/cmake/modules/CGAL_SetupCGAL_CoreDependencies.cmake @@ -53,13 +53,6 @@ endif() # keyword, or ``PUBLIC`` otherwise. # -# See the release notes of CGAL-4.10: CGAL_Core now requires -# Boost.Thread, with GNU G++. -if (CMAKE_CXX_COMPILER_ID STREQUAL GNU) - include(${CMAKE_CURRENT_LIST_DIR}/CGAL_TweakFindBoost.cmake) - find_package( Boost 1.48 REQUIRED COMPONENTS thread system ) -endif() - function(CGAL_setup_CGAL_Core_dependencies target) if(ARGV1 STREQUAL INTERFACE) set(keyword INTERFACE) @@ -70,21 +63,4 @@ function(CGAL_setup_CGAL_Core_dependencies target) use_CGAL_GMP_support(CGAL_Core ${keyword}) target_compile_definitions(${target} ${keyword} CGAL_USE_CORE=1) target_link_libraries( CGAL_Core ${keyword} CGAL::CGAL ) - - # See the release notes of CGAL-4.10: CGAL_Core now requires - # Boost.Thread, with GNU G++. - if (CMAKE_CXX_COMPILER_ID STREQUAL GNU) - if(TARGET Boost::thread) - target_link_libraries( CGAL_Core ${keyword} Boost::thread) - else() - # Note that `find_package( Boost...)` must be called in the - # function `CGAL_setup_CGAL_Core_dependencies()` because the - # calling `CMakeLists.txt` may also call `find_package(Boost)` - # between the inclusion of this module, and the call to this - # function. That resets `Boost_LIBRARIES`. - find_package( Boost 1.48 REQUIRED thread system ) - target_link_libraries( CGAL_Core ${keyword} ${Boost_LIBRARIES}) - endif() - endif() - endfunction() From 7b0c6f0cf712e5b4122f3bca10bda2ad6a294d1c Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Tue, 18 Jun 2019 11:28:14 +0200 Subject: [PATCH 2/6] Fix a stack-use-after-scope in Arr_polycurve_basic_traits_2 Before this patch, `Arr_polycurve_basic_traits_2` was holding a const reference to a temporary object. Here was the result of ctest, with the address sanitizer: ```shellsession $ make && ASAN_OPTIONS=detect_leaks=0 ctest -L Arr -j6 [...] The following tests FAILED: 144 - execution___of__generic_curve_data (Failed) 170 - execution___of__polycurve_circular_arc (Failed) 172 - execution___of__polycurve_conic (Failed) 176 - execution___of__polycurves_basic (Failed) 178 - execution___of__polylines (Failed) 2166 - test_traits_test_polylines__data_polylines_compare_y_at_x__polyline_traits (Failed) 2168 - test_traits_test_polylines__data_polylines_intersect__polyline_traits (Failed) 2169 - test_traits_test_polylines__data_polylines_split__polyline_traits (Failed) 2171 - test_traits_test_polylines__data_polylines_assertions__polyline_traits (Failed) 2175 - test_traits_conic_polycurve__data_polycurves_conics_compare_y_at_x__polycurve_conic_traits (Failed) 2176 - test_traits_conic_polycurve__data_polycurves_conics_compare_y_at_x_left__polycurve_conic_traits (Failed) 2177 - test_traits_conic_polycurve__data_polycurves_conics_compare_y_at_x_right__polycurve_conic_traits (Failed) 2179 - test_traits_conic_polycurve__data_polycurves_conics_intersect__polycurve_conic_traits (Failed) 2180 - test_traits_conic_polycurve__data_polycurves_conics_split__polycurve_conic_traits (Failed) 2193 - test_traits_circular_arc_polycurve__data_Polycurves_circular_arcs_compare_y_at_x__polycurve_circular_arc_traits (Failed) 2198 - test_traits_circular_arc_polycurve__data_Polycurves_circular_arcs_split__polycurve_circular_arc_traits (Failed) 2221 - test_traits_non_caching_polylines__data_polylines_compare_y_at_x__non_caching_polyline_traits (Failed) 2223 - test_traits_non_caching_polylines__data_polylines_intersect__non_caching_polyline_traits (Failed) 2224 - test_traits_non_caching_polylines__data_polylines_split__non_caching_polyline_traits (Failed) 2226 - test_traits_non_caching_polylines__data_polylines_assertions__non_caching_polyline_traits (Failed) 2400 - execution___of__test_construction_polylines (Failed) 2477 - execution___of__test_io (Failed) ``` This is fixed rather easily, by copying the traits class, instead of holding a reference to it. --- .../include/CGAL/Arr_polycurve_basic_traits_2.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Arrangement_on_surface_2/include/CGAL/Arr_polycurve_basic_traits_2.h b/Arrangement_on_surface_2/include/CGAL/Arr_polycurve_basic_traits_2.h index 5fc50e0c5fb..da39fc3a02c 100644 --- a/Arrangement_on_surface_2/include/CGAL/Arr_polycurve_basic_traits_2.h +++ b/Arrangement_on_surface_2/include/CGAL/Arr_polycurve_basic_traits_2.h @@ -2534,7 +2534,7 @@ protected: Polycurve_basic_traits_2; /*! The polycurve traits (in case it has state). */ - const Polycurve_basic_traits_2& m_poly_traits; + const Polycurve_basic_traits_2 m_poly_traits; const Point_2& m_point; From 1bcaf6bbf3ffde79451100b9ef77e9631c520199 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Thu, 20 Jun 2019 11:57:37 +0200 Subject: [PATCH 3/6] Fix in non-header-only: one has to call find_package(Boost) --- .../cmake/modules/CGALConfig_binary.cmake.in | 3 +++ .../cmake/modules/CGALConfig_install.cmake.in | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/Installation/cmake/modules/CGALConfig_binary.cmake.in b/Installation/cmake/modules/CGALConfig_binary.cmake.in index abc58be24ed..e22b4141f3a 100644 --- a/Installation/cmake/modules/CGALConfig_binary.cmake.in +++ b/Installation/cmake/modules/CGALConfig_binary.cmake.in @@ -12,6 +12,9 @@ get_filename_component(CGAL_CONFIG_DIR "${CMAKE_CURRENT_LIST_FILE}" PATH) set(CGAL_HEADER_ONLY "@CGAL_HEADER_ONLY@" ) +include(CMakeFindDependencyMacro) +find_dependency(Boost REQUIRED) + # The code for including exported targets is different from # CGAL_Config_install.cmake. We do not have separate export files in # an installed version and we need to make sure that we are not diff --git a/Installation/cmake/modules/CGALConfig_install.cmake.in b/Installation/cmake/modules/CGALConfig_install.cmake.in index 17606ef1ddd..91f7a645385 100644 --- a/Installation/cmake/modules/CGALConfig_install.cmake.in +++ b/Installation/cmake/modules/CGALConfig_install.cmake.in @@ -12,6 +12,9 @@ get_filename_component(CGAL_CONFIG_DIR "${CMAKE_CURRENT_LIST_FILE}" PATH) set(CGAL_HEADER_ONLY "@CGAL_HEADER_ONLY@" ) +include(CMakeFindDependencyMacro) +find_dependency(Boost REQUIRED) + # CGAL_DIR is the directory where this CGALConfig.cmake is installed string(REPLACE "/@CGAL_INSTALL_CMAKE_DIR@" "" CGAL_INSTALL_PREFIX "${CGAL_CONFIG_DIR}") @@ -82,6 +85,15 @@ macro(check_cgal_component COMPONENT) # include config file include(${CGAL_CONFIG_DIR}/CGALLibConfig.cmake) set( CHECK_CGAL_ERROR_TAIL "" ) + get_property(CGAL_CGAL_is_imported TARGET CGAL::CGAL PROPERTY IMPORTED) + if(CGAL_CGAL_is_imported) + include("${CGAL_MODULES_DIR}/CGAL_SetupBoost.cmake") + get_property(CGAL_requires_Boost_libs + GLOBAL PROPERTY CGAL_requires_Boost_Thread) + if(CGAL_requires_Boost_libs AND TARGET Boost::thread) + set_property(TARGET CGAL::CGAL APPEND PROPERTY INTERFACE_LINK_LIBRARIES Boost::thread) + endif() + endif() else() if (EXISTS ${CGAL_CONFIG_DIR}/${CGAL_LIB}Exports.cmake) # include export files for requested component From 2ce7800239e7924f560c49bd569c81910eaaa37c Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Mon, 24 Jun 2019 11:59:23 +0200 Subject: [PATCH 4/6] Unfortunately gcc < 9.1 has bug with thread_local that impact CGAL --- CGAL_Core/include/CGAL/CORE/MemoryPool.h | 35 +++++++++++- .../doc/Documentation/Installation.txt | 5 ++ .../CGAL_SetupCGAL_CoreDependencies.cmake | 24 ++++++++ Installation/include/CGAL/tss.h | 56 +++---------------- 4 files changed, 68 insertions(+), 52 deletions(-) diff --git a/CGAL_Core/include/CGAL/CORE/MemoryPool.h b/CGAL_Core/include/CGAL/CORE/MemoryPool.h index 64121773f97..596613a727b 100644 --- a/CGAL_Core/include/CGAL/CORE/MemoryPool.h +++ b/CGAL_Core/include/CGAL/CORE/MemoryPool.h @@ -38,6 +38,13 @@ #include #include #include +#if defined(CGAL_HAS_THREADS) && defined(BOOST_GCC) && BOOST_GCC < 90100 +// Force the use of Boost.Thread with g++ and C++11, because of the PR66944 +// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66944 +// See also CGAL PR #1888 +// https://github.com/CGAL/cgal/pull/1888#issuecomment-278284232 +# include +#endif #include // for placement new #include @@ -69,7 +76,7 @@ public: t = t->next; } //); - //Cgal_warning_msg(count == nObjects * blocks.size(), + //CGAL_warning_msg(count == nObjects * blocks.size(), // "Cannot delete memory as there are cyclic references"); if(count == nObjects * blocks.size()){ @@ -85,16 +92,38 @@ public: // Access the corresponding static global allocator. static MemoryPool& global_allocator() { - CGAL_STATIC_THREAD_LOCAL_VARIABLE_0(Self, memPool); +#if defined(CGAL_HAS_THREADS) && defined(BOOST_GCC) && BOOST_GCC < 90100 + if(memPool_ptr.get() == nullptr) {memPool_ptr.reset(new Self());} + Self& memPool = * memPool_ptr.get(); +#endif return memPool; } private: - Thunk* head; // next available block in the pool + Thunk* head; // next available block in the pool std::vector blocks; +#if defined(CGAL_HAS_THREADS) && defined(BOOST_GCC) && BOOST_GCC < 90100 + static boost::thread_specific_ptr memPool_ptr; +#elif defined(CGAL_HAS_THREADS) // use the C++11 implementation + static thread_local Self memPool; +#else // not CGAL_HAS_THREADS + static Self memPool; +#endif // not CGAL_HAS_THREADS }; +#if defined(CGAL_HAS_THREADS) && defined(BOOST_GCC) && BOOST_GCC < 90100 +template +boost::thread_specific_ptr > +MemoryPool::memPool_ptr; +#else // use C++11 or without CGAL_HAS_THREADS +template +# ifdef CGAL_HAS_THREADS +thread_local +# endif +MemoryPool MemoryPool::memPool; +#endif + template< class T, int nObjects > void* MemoryPool< T, nObjects >::allocate(std::size_t) { if ( head == 0 ) { // if no more memory in the pool diff --git a/Documentation/doc/Documentation/Installation.txt b/Documentation/doc/Documentation/Installation.txt index e8754dbcdb4..f6a6b8e6ee0 100644 --- a/Documentation/doc/Documentation/Installation.txt +++ b/Documentation/doc/Documentation/Installation.txt @@ -390,6 +390,11 @@ installed as binaries. \cgal only requires the headers of the \sc{Boost} libraries, but some demos and examples depend on the binary library `Boost.Program_options`. +As an exception, because of a bug in the \gcc compiler about the \cpp 11 +keyword `thread_local`, the `CGAL_Core` library always requires +the binary library `Boost.Thread` if the \gcc compiler version 9.0 or +earlier is used. + In case the \sc{Boost} libraries are not installed on your system already, you can obtain them from `https://www.boost.org/`. For Visual C++ you can download precompiled libraries from `https://sourceforge.net/projects/boost/files/boost-binaries/`. diff --git a/Installation/cmake/modules/CGAL_SetupCGAL_CoreDependencies.cmake b/Installation/cmake/modules/CGAL_SetupCGAL_CoreDependencies.cmake index 609f086694b..053c4f6ab67 100644 --- a/Installation/cmake/modules/CGAL_SetupCGAL_CoreDependencies.cmake +++ b/Installation/cmake/modules/CGAL_SetupCGAL_CoreDependencies.cmake @@ -53,6 +53,13 @@ endif() # keyword, or ``PUBLIC`` otherwise. # +# See the release notes of CGAL-4.10: CGAL_Core now requires +# Boost.Thread, with GNU G++ < 9.1. +if (CMAKE_CXX_COMPILER_ID STREQUAL GNU AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9.1) + include(${CMAKE_CURRENT_LIST_DIR}/CGAL_TweakFindBoost.cmake) + find_package( Boost 1.48 REQUIRED COMPONENTS thread system ) +endif() + function(CGAL_setup_CGAL_Core_dependencies target) if(ARGV1 STREQUAL INTERFACE) set(keyword INTERFACE) @@ -63,4 +70,21 @@ function(CGAL_setup_CGAL_Core_dependencies target) use_CGAL_GMP_support(CGAL_Core ${keyword}) target_compile_definitions(${target} ${keyword} CGAL_USE_CORE=1) target_link_libraries( CGAL_Core ${keyword} CGAL::CGAL ) + + # See the release notes of CGAL-4.10: CGAL_Core now requires + # Boost.Thread, with GNU G++. + if (CMAKE_CXX_COMPILER_ID STREQUAL GNU AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9.1) + if(TARGET Boost::thread) + target_link_libraries( CGAL_Core ${keyword} Boost::thread) + else() + # Note that `find_package( Boost...)` must be called in the + # function `CGAL_setup_CGAL_Core_dependencies()` because the + # calling `CMakeLists.txt` may also call `find_package(Boost)` + # between the inclusion of this module, and the call to this + # function. That resets `Boost_LIBRARIES`. + find_package( Boost 1.48 REQUIRED thread system ) + target_link_libraries( CGAL_Core ${keyword} ${Boost_LIBRARIES}) + endif() + endif() + endfunction() diff --git a/Installation/include/CGAL/tss.h b/Installation/include/CGAL/tss.h index 7096b1612e8..22f47a46632 100644 --- a/Installation/include/CGAL/tss.h +++ b/Installation/include/CGAL/tss.h @@ -22,64 +22,22 @@ #include #if defined( CGAL_HAS_THREADS ) -# ifdef CGAL_CAN_USE_CXX11_THREAD_LOCAL -//# pragma message ( "Use keyword thread_local" ) -# else -//# pragma message ("Use thread_local from boost") -# define CGAL_USE_BOOST_THREAD -# include -# endif +# define CGAL_STATIC_THREAD_LOCAL_VARIABLE_0(TYPE, VAR) \ + static thread_local TYPE VAR -# ifdef CGAL_USE_BOOST_THREAD -# define CGAL_STATIC_THREAD_LOCAL_USE_BOOST 1 +# define CGAL_STATIC_THREAD_LOCAL_VARIABLE(TYPE, VAR, ARG1) \ + static thread_local TYPE VAR(ARG1) -# define CGAL_STATIC_THREAD_LOCAL_VARIABLE_0(TYPE, VAR) \ - static boost::thread_specific_ptr VAR##_ptr; \ - if(VAR##_ptr.get() == nullptr) {VAR##_ptr.reset(new TYPE());} \ - TYPE& VAR = * VAR##_ptr.get() - -# define CGAL_STATIC_THREAD_LOCAL_VARIABLE(TYPE, VAR, ARG1) \ - static boost::thread_specific_ptr VAR##_ptr; \ - if(VAR##_ptr.get() == nullptr) {VAR##_ptr.reset(new TYPE(ARG1));} \ - TYPE& VAR = * VAR##_ptr.get() - -# define CGAL_STATIC_THREAD_LOCAL_VARIABLE_2(TYPE, VAR, ARG1, ARG2) \ - static boost::thread_specific_ptr VAR##_ptr; \ - if(VAR##_ptr.get() == nullptr) {VAR##_ptr.reset(new TYPE(ARG1,ARG2));} \ - TYPE& VAR = * VAR##_ptr.get() - -# define CGAL_STATIC_THREAD_LOCAL_VARIABLE_3(TYPE, VAR, ARG1, ARG2, ARG3) \ - static boost::thread_specific_ptr VAR##_ptr; \ - if(VAR##_ptr.get() == nullptr) {VAR##_ptr.reset(new TYPE(ARG1,ARG2,ARG3));} \ - TYPE& VAR = * VAR##_ptr.get() - -# define CGAL_STATIC_THREAD_LOCAL_VARIABLE_4(TYPE, VAR, ARG1, ARG2, ARG3, ARG4) \ - static boost::thread_specific_ptr VAR##_ptr; \ - if(VAR##_ptr.get() == nullptr) {VAR##_ptr.reset(new TYPE(ARG1,ARG2,ARG3,ARG4));} \ - TYPE& VAR = * VAR##_ptr.get() - - - -# else // not CGAL_USE_BOOST_THREAD, -> use C++11 thread_local - -# define CGAL_STATIC_THREAD_LOCAL_VARIABLE_0(TYPE, VAR) \ - static thread_local TYPE VAR - -# define CGAL_STATIC_THREAD_LOCAL_VARIABLE(TYPE, VAR, ARG1) \ - static thread_local TYPE VAR(ARG1) - -# define CGAL_STATIC_THREAD_LOCAL_VARIABLE_2(TYPE, VAR, ARG1, ARG2) \ +# define CGAL_STATIC_THREAD_LOCAL_VARIABLE_2(TYPE, VAR, ARG1, ARG2) \ static thread_local TYPE VAR(ARG1,ARG2) -# define CGAL_STATIC_THREAD_LOCAL_VARIABLE_3(TYPE, VAR, ARG1, ARG2, ARG3) \ +# define CGAL_STATIC_THREAD_LOCAL_VARIABLE_3(TYPE, VAR, ARG1, ARG2, ARG3) \ static thread_local TYPE VAR(ARG1,ARG2,ARG3) -# define CGAL_STATIC_THREAD_LOCAL_VARIABLE_4(TYPE, VAR, ARG1, ARG2, ARG3, ARG4) \ +# define CGAL_STATIC_THREAD_LOCAL_VARIABLE_4(TYPE, VAR, ARG1, ARG2, ARG3, ARG4) \ static thread_local TYPE VAR(ARG1,ARG2,ARG3,ARG4) -# endif // not CGAL_USE_BOOST_THREAD - #else // not CGAL_HAS_THREADS # define CGAL_STATIC_THREAD_LOCAL_VARIABLE_0(TYPE, VAR) static TYPE VAR From 9eed06477dfe366ee08fde1eb22dcd0e51a77666 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Wed, 26 Jun 2019 15:51:43 +0200 Subject: [PATCH 5/6] Use the function-local thread_local memPool, but for gcc<9.1 --- CGAL_Core/include/CGAL/CORE/MemoryPool.h | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/CGAL_Core/include/CGAL/CORE/MemoryPool.h b/CGAL_Core/include/CGAL/CORE/MemoryPool.h index 596613a727b..d6a49142c67 100644 --- a/CGAL_Core/include/CGAL/CORE/MemoryPool.h +++ b/CGAL_Core/include/CGAL/CORE/MemoryPool.h @@ -95,7 +95,11 @@ public: #if defined(CGAL_HAS_THREADS) && defined(BOOST_GCC) && BOOST_GCC < 90100 if(memPool_ptr.get() == nullptr) {memPool_ptr.reset(new Self());} Self& memPool = * memPool_ptr.get(); -#endif +#elif defined(CGAL_HAS_THREADS) // use the C++11 implementation + static thread_local Self memPool; +#else // not CGAL_HAS_THREADS + static Self memPool; +#endif // not CGAL_HAS_THREADS return memPool; } @@ -105,10 +109,6 @@ private: #if defined(CGAL_HAS_THREADS) && defined(BOOST_GCC) && BOOST_GCC < 90100 static boost::thread_specific_ptr memPool_ptr; -#elif defined(CGAL_HAS_THREADS) // use the C++11 implementation - static thread_local Self memPool; -#else // not CGAL_HAS_THREADS - static Self memPool; #endif // not CGAL_HAS_THREADS }; @@ -116,12 +116,6 @@ private: template boost::thread_specific_ptr > MemoryPool::memPool_ptr; -#else // use C++11 or without CGAL_HAS_THREADS -template -# ifdef CGAL_HAS_THREADS -thread_local -# endif -MemoryPool MemoryPool::memPool; #endif template< class T, int nObjects > From dca6b5c19eff164799eb2f803b8776d3fa0f10e8 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Tue, 2 Jul 2019 10:53:09 +0200 Subject: [PATCH 6/6] Strange patch that fixes the remaining bugs https://github.com/CGAL/cgal/pull/4013#issuecomment-507291311 Because of two constructions in Algebraic_curve_kernel_2 and Hyperbolic_octagon_translation, the assertion "!blocks.empty()" from `` was triggered during the destruction of thread-local objects. This strange patch ensures that the order of creation of thread-local object is right, so the order of destruction is right as well. --- .../Algebraic_curve_kernel_2.h | 12 +++++++ CGAL_Core/include/CGAL/CORE/MemoryPool.h | 4 +++ .../CGAL/Hyperbolic_octagon_translation.h | 34 ++++++++++++++----- 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/Algebraic_kernel_d/include/CGAL/Algebraic_kernel_d/Algebraic_curve_kernel_2.h b/Algebraic_kernel_d/include/CGAL/Algebraic_kernel_d/Algebraic_curve_kernel_2.h index bcc17651c39..8f2e79215d3 100644 --- a/Algebraic_kernel_d/include/CGAL/Algebraic_kernel_d/Algebraic_curve_kernel_2.h +++ b/Algebraic_kernel_d/include/CGAL/Algebraic_kernel_d/Algebraic_curve_kernel_2.h @@ -408,7 +408,19 @@ public: } public: + static auto initialize_poly_0() { + Polynomial_2 poly_0; + return poly_0; + } static Algebraic_curve_kernel_2& get_static_instance(){ + // Useless reference to a `Polynomial_2` to force the creation + // of `CORE::MemoryPool` (and related type) + // before the static thread-local instance `ack_2_instance`. + // The issue is otherwise that the memory pool is created during + // the filling of the curves cache, and then destroyed too soon, + // before the destruction of `ack_2_instance`. + CGAL_STATIC_THREAD_LOCAL_VARIABLE(Polynomial_2, poly_0, initialize_poly_0()); + CGAL_USE(poly_0); // a default constructed ack_2 instance CGAL_STATIC_THREAD_LOCAL_VARIABLE_0(Algebraic_curve_kernel_2, ack_2_instance); return ack_2_instance; diff --git a/CGAL_Core/include/CGAL/CORE/MemoryPool.h b/CGAL_Core/include/CGAL/CORE/MemoryPool.h index d6a49142c67..b033bc5ecfd 100644 --- a/CGAL_Core/include/CGAL/CORE/MemoryPool.h +++ b/CGAL_Core/include/CGAL/CORE/MemoryPool.h @@ -84,6 +84,10 @@ public: ::operator delete(blocks[i]); } } + // un-commenting the following line can help reproduce on Linux the + // assertion !blocks.empty() that is sometimes triggered with MSVC + // or AppleClang + // blocks.clear(); } diff --git a/Periodic_4_hyperbolic_triangulation_2/include/CGAL/Hyperbolic_octagon_translation.h b/Periodic_4_hyperbolic_triangulation_2/include/CGAL/Hyperbolic_octagon_translation.h index 36dde2fe321..c324efa94e5 100644 --- a/Periodic_4_hyperbolic_triangulation_2/include/CGAL/Hyperbolic_octagon_translation.h +++ b/Periodic_4_hyperbolic_triangulation_2/include/CGAL/Hyperbolic_octagon_translation.h @@ -59,15 +59,8 @@ private: Word _wrd; - - - static const Matrix& gmap(const std::string& s) - { - typedef std::map M; - CGAL_STATIC_THREAD_LOCAL_VARIABLE_0(M, m); - - if(m.empty()){ - + static auto initialize_gmap() { + std::map m; std::vector g; Matrix::generators(g); @@ -128,7 +121,30 @@ private: m["7"] = g[D]; m["72"] = g[D]*g[C]; m["725"] = g[D]*g[C]*g[B]; + + { // This block abuses `operator<<` of numbers, to a null stream. + // That ensures that the following memory pool are correctly + // initialized: + // - `CORE::MemoryPool` + // - `CORE::MemoryPool` + // - `CORE::MemoryPool` + // - `CORE::MemoryPool` + // otherwise, there is an assertion during the destruction of + // static (or `thread_local`) objects + struct NullBuffer : public std::streambuf { + int overflow(int c) { return c; } + }; + NullBuffer null_buffer; + std::ostream null_stream(&null_buffer); + for(auto& pair: m) null_stream << pair.second; } + return m; + } + + static const Matrix& gmap(const std::string& s) + { + typedef std::map M; + CGAL_STATIC_THREAD_LOCAL_VARIABLE(M, m, initialize_gmap()); return m[s]; }