This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new 0e8a9077b IMPALA-12915: Use libgtest.so when built with shared libs
0e8a9077b is described below
commit 0e8a9077b10ff478e0861a1a266a38927a5ae544
Author: stiga-huang <[email protected]>
AuthorDate: Tue Mar 19 15:08:58 2024 +0800
IMPALA-12915: Use libgtest.so when built with shared libs
When building Impala with shared libraries, we currently link against
the static library of GTest in both libkudu_test_util.so and
unifiedbetests. libkudu_test_util.so is also linked by unifiedbetests
dynamically. When unifiedbetests exits, both binaries try to delete the
global variables of GTest, which leads to a double-free memory issue.
To fix the double-free memory issue, instead of statically linking
libgtest.a, all binaries should dynamically link libgtest.so. So when
the process exits, only libgtest.so will delete its global variables.
Tests
- Verified the issue resolved locally
Change-Id: I27d21217db219f52b072a4e5cfa1caaace35d1a2
Reviewed-on: http://gerrit.cloudera.org:8080/21163
Reviewed-by: Impala Public Jenkins <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
---
CMakeLists.txt | 2 +-
bin/impala-config.sh | 6 +++---
cmake_modules/FindGTest.cmake | 21 +++++++++++++++------
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index b61982d7a..dcdf9d103 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -256,7 +256,7 @@ IMPALA_ADD_THIRDPARTY_LIB(pprof ${PPROF_INCLUDE_DIR}
${PPROF_STATIC_LIB} "")
# find GTest headers and libs
set (GTEST_FIND_QUIETLY TRUE)
find_package(GTest REQUIRED)
-IMPALA_ADD_THIRDPARTY_LIB(gtest ${GTEST_INCLUDE_DIR} ${GTEST_STATIC_LIB} "")
+IMPALA_ADD_THIRDPARTY_LIB(gtest ${GTEST_INCLUDE_DIR} ${GTEST_STATIC_LIB}
${GTEST_SHARED_LIB})
# Use LLVM release binaries.
set(LLVM_BINARIES_ROOT ${LLVM_ROOT})
diff --git a/bin/impala-config.sh b/bin/impala-config.sh
index b023088bf..98c9dda46 100755
--- a/bin/impala-config.sh
+++ b/bin/impala-config.sh
@@ -85,13 +85,13 @@ export USE_AVRO_CPP=${USE_AVRO_CPP:=false}
# moving to a different build of the toolchain, e.g. when a version is bumped
or a
# compile option is changed. The build id can be found in the output of the
toolchain
# build jobs, it is constructed from the build number and toolchain git hash
prefix.
-export IMPALA_TOOLCHAIN_BUILD_ID_AARCH64=11-8dbe785e9e
-export IMPALA_TOOLCHAIN_BUILD_ID_X86_64=380-8dbe785e9e
+export IMPALA_TOOLCHAIN_BUILD_ID_AARCH64=25-051b912729
+export IMPALA_TOOLCHAIN_BUILD_ID_X86_64=413-051b912729
export IMPALA_TOOLCHAIN_REPO=\
${IMPALA_TOOLCHAIN_REPO:-https://github.com/cloudera/native-toolchain.git}
export IMPALA_TOOLCHAIN_BRANCH=${IMPALA_TOOLCHAIN_BRANCH:-master}
export IMPALA_TOOLCHAIN_COMMIT_HASH=\
-${IMPALA_TOOLCHAIN_COMMIT_HASH-8dbe785e9e000c665722a11068f9a4a6b454cfeb}
+${IMPALA_TOOLCHAIN_COMMIT_HASH-051b912729cdf327572fec5006c2df054b857f50}
# Compare the build ref in build IDs by removing everything
up-to-and-including the
# first hyphen.
if [ "${IMPALA_TOOLCHAIN_BUILD_ID_AARCH64#*-}" \
diff --git a/cmake_modules/FindGTest.cmake b/cmake_modules/FindGTest.cmake
index b3d313402..55c83119e 100644
--- a/cmake_modules/FindGTest.cmake
+++ b/cmake_modules/FindGTest.cmake
@@ -28,7 +28,8 @@
# GTEST_INCLUDE_DIR, where to find gtest include files, etc.
# GTEST_LIBRARIES, the libraries to link against to use gtest.
# GTEST_FOUND, If false, do not try to use gtest.
-# GTEST_STATIC_LIB, where to find the GTest library.
+# GTEST_STATIC_LIB, path to libgtest.a.
+# GTEST_SHARED_LIB, path to libgtest.so.
set(GTEST_H gtest/gtest.h)
@@ -44,18 +45,24 @@ find_library(GTEST_STATIC_LIB NAMES libgtest.a
DOC "Google's framework for writing C++ tests (gtest)"
)
+find_library(GTEST_SHARED_LIB NAMES libgtest.so
+ PATHS ${GTEST_ROOT}/lib
+ NO_DEFAULT_PATH
+ DOC "Google's framework for writing C++ tests (gtest)"
+)
+
find_library(GTEST_MAIN_LIBRARY NAMES libgtest_main.a
PATHS ${GTEST_ROOT}/lib
NO_DEFAULT_PATH
DOC "Google's framework for writing C++ tests (gtest_main)"
)
-if(GTEST_INCLUDE_DIR AND GTEST_STATIC_LIB AND GTEST_MAIN_LIBRARY)
- set(GTEST_LIBRARIES ${GTEST_STATIC_LIB} ${GTEST_MAIN_LIBRARY})
+if(GTEST_INCLUDE_DIR AND GTEST_STATIC_LIB AND GTEST_SHARED_LIB AND
GTEST_MAIN_LIBRARY)
+ set(GTEST_LIBRARIES ${GTEST_STATIC_LIB} ${GTEST_SHARED_LIB}
${GTEST_MAIN_LIBRARY})
set(GTEST_FOUND TRUE)
-else(GTEST_INCLUDE_DIR AND GTEST_STATIC_LIB AND GTEST_MAIN_LIBRARY)
+else(GTEST_INCLUDE_DIR AND GTEST_STATIC_LIB AND GTEST_SHARED_LIB AND
GTEST_MAIN_LIBRARY)
set(GTEST_FOUND FALSE)
-endif(GTEST_INCLUDE_DIR AND GTEST_STATIC_LIB AND GTEST_MAIN_LIBRARY)
+endif(GTEST_INCLUDE_DIR AND GTEST_STATIC_LIB AND GTEST_SHARED_LIB AND
GTEST_MAIN_LIBRARY)
if(GTEST_FOUND)
if(NOT GTEST_FIND_QUIETLY)
@@ -68,4 +75,6 @@ endif(GTEST_FOUND)
mark_as_advanced(
GTEST_INCLUDE_DIR
GTEST_LIBRARIES
- GTEST_STATIC_LIB)
+ GTEST_STATIC_LIB
+ GTEST_SHARED_LIB
+)
