Skip to content

Commit

Permalink
Fixed ASAN issue in NamedThread (vesoft-inc#60)
Browse files Browse the repository at this point in the history
  • Loading branch information
dutor authored Jan 4, 2019
1 parent e7c87b9 commit e827c93
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 6 deletions.
8 changes: 5 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ if(NOT ${NEBULA_GPERF_BIN_DIR} STREQUAL "")
endif()


option(sanitize "Whether to turn AddressSanitizer ON or OFF" OFF)
option(asan "Whether to turn AddressSanitizer ON or OFF" OFF)

message(STATUS "ASAN: ${asan}")


message(STATUS "CMAKE_INCLUDE_PATH: " ${CMAKE_INCLUDE_PATH})
Expand All @@ -107,7 +109,7 @@ find_package(Libunwind REQUIRED)
find_package(BISON REQUIRED)
find_package(FLEX REQUIRED)
find_package(Readline REQUIRED)
if(NOT sanitize)
if(NOT asan)
find_package(PCHSupport)
add_compile_options(-Winvalid-pch)
endif()
Expand All @@ -118,7 +120,7 @@ add_compile_options(-Werror)
add_compile_options(-Wunused-parameter)
add_compile_options(-Wshadow)

if(sanitize)
if(asan)
add_compile_options(-fsanitize=address)
add_compile_options(-g)
add_compile_options(-fno-omit-frame-pointer)
Expand Down
11 changes: 9 additions & 2 deletions src/common/thread/NamedThread.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,17 @@ class NamedThread final : public std::thread {

pid_t tid() const {
while (tid_ == 0) {
// `tid' is unavailable until the thread function is called.
// `tid_' is unavailable until the thread function is called.
}
return tid_;
}

// Busy waiting for the thread to enter the thread function
void waitForRunning() const {
// Use `tid()' for our purpose
tid();
}

public:
class Nominator {
public:
Expand Down Expand Up @@ -75,7 +81,8 @@ class NamedThread final : public std::thread {
}

private:
pid_t tid_{0};
// `volatile' to make the change visible in `tid()'
volatile pid_t tid_{0};
};

template <typename F, typename...Args>
Expand Down
8 changes: 8 additions & 0 deletions src/common/time/Duration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ void launchTickTockThread() {
ticksPerUSec.store(calibrateTicksPerUSec());
} // while
});

// Since `t' would be destructed instantly when this function returns.
// We have to make sure that the thread function has been invoked.
// Otherwise, accessing `NamedThread::tid_' in the the `NamedThread::hook' might
// cause a `stack-buffer-overflow' error under ASAN.
// See `src/common/thread/NamedThread.h' for details.
t.waitForRunning();

t.detach();
}

Expand Down
2 changes: 1 addition & 1 deletion third-party/rocksdb/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ NEBULA_LIB_DIRS="$zlib_release/lib;$zstd_release/lib;$snappy_release/lib;$jemall

if [[ $SOURCE_DIR/CMakeLists.txt -nt $SOURCE_DIR/Makefile ||
$CURR_DIR/build.sh -nt $SOURCE_DIR/Makefile ]]; then
if !($NEBULA_CMAKE $CMAKE_FLAGS -DCMAKE_C_FLAGS:STRING="${compiler_flags}" -DCMAKE_CXX_FLAGS:STRING="${compiler_flags}" -DCMAKE_EXE_LINKER_FLAGS:STRING="${exe_linker_flags}" -DCMAKE_INCLUDE_PATH="$NEBULA_INCLUDE_DIRS" -DCMAKE_LIBRARY_PATH="$NEBULA_LIB_DIRS" -DCMAKE_PROGRAM_PATH="$NEBULA_PROGRAM_DIRS" -DWITH_ZLIB=1 -DWITH_SNAPPY=1 -DWITH_ZSTD=1 -DWITH_GFLAGS=0 -DWITH_JEMALLOC=1 -DWITH_TESTS=off -DWITH_TOOLS=off .); then
if !($NEBULA_CMAKE $CMAKE_FLAGS -DCMAKE_C_FLAGS:STRING="${compiler_flags}" -DCMAKE_CXX_FLAGS:STRING="${compiler_flags}" -DCMAKE_EXE_LINKER_FLAGS:STRING="${exe_linker_flags}" -DCMAKE_INCLUDE_PATH="$NEBULA_INCLUDE_DIRS" -DCMAKE_LIBRARY_PATH="$NEBULA_LIB_DIRS" -DCMAKE_PROGRAM_PATH="$NEBULA_PROGRAM_DIRS" -DWITH_ZLIB=1 -DWITH_SNAPPY=1 -DWITH_ZSTD=1 -DWITH_GFLAGS=0 -DWITH_JEMALLOC=0 -DWITH_TESTS=off -DWITH_TOOLS=off .); then
cd $CURR_DIR
echo
echo "### $PROJECT_NAME failed to install ###"
Expand Down

0 comments on commit e827c93

Please sign in to comment.