diff --git a/.github/workflows/ci-linux.yml b/.github/workflows/ci-linux.yml index 1854483a42..eeccf1cb49 100644 --- a/.github/workflows/ci-linux.yml +++ b/.github/workflows/ci-linux.yml @@ -14,154 +14,6 @@ env: proc_num: $(nproc) jobs: - gcc-compile-with-make: - runs-on: ubuntu-20.04 # https://github.com/actions/runner-images - steps: - - uses: actions/checkout@v2 - - uses: ./.github/actions/install-essential-dependences - - uses: ./.github/actions/init-make-config - with: - options: --cc=gcc --cxx=g++ - - name: compile - run: | - make -j ${{env.proc_num}} - - gcc-compile-with-cmake: - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - - uses: ./.github/actions/install-essential-dependences - - name: cmake - run: | - export CC=gcc && export CXX=g++ - mkdir build - cd build - cmake .. - - name: compile - run: | - cd build - make -j ${{env.proc_num}} - - gcc-compile-with-bazel: - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - - run: bazel test --verbose_failures -- //... -//example/... - - gcc-compile-with-boringssl: - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - - run: bazel test --verbose_failures --define with_mesalink=false --define with_glog=true --define with_thrift=true --define BRPC_WITH_BORINGSSL=true -- //... -//example/... - - gcc-compile-with-make-all-options: - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - - uses: ./.github/actions/install-all-dependences - - uses: ./.github/actions/init-make-config - with: - options: --cc=gcc --cxx=g++ --with-thrift --with-glog --with-rdma --with-debug-bthread-sche-safety --with-debug-lock --with-bthread-tracer - - name: compile - run: | - make -j ${{env.proc_num}} - - gcc-compile-with-cmake-all-options: - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - - uses: ./.github/actions/install-all-dependences - - name: cmake - run: | - export CC=gcc && export CXX=g++ - mkdir build - cd build - cmake -DWITH_MESALINK=OFF -DWITH_GLOG=ON -DWITH_THRIFT=ON -DWITH_RDMA=ON -DWITH_DEBUG_BTHREAD_SCHE_SAFETY=ON -DWITH_DEBUG_LOCK=ON -WITH_BTHREAD_TRACER=ON .. - - name: compile - run: | - cd build - make -j ${{env.proc_num}} - - gcc-compile-with-bazel-all-options: - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - - run: bazel test --verbose_failures --define with_mesalink=false --define with_glog=true --define with_thrift=true --define with_debug_bthread_sche_safety=true --define with_debug_lock=true -- //... -//example/... - - clang-compile-with-make: - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - - uses: ./.github/actions/install-essential-dependences - - uses: ./.github/actions/init-make-config - with: - options: --cc=clang --cxx=clang++ - - name: compile - run: | - make -j ${{env.proc_num}} - - clang-compile-with-cmake: - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - - uses: ./.github/actions/install-essential-dependences - - name: cmake - run: | - export CC=clang && export CXX=clang++ - mkdir build - cd build - cmake .. - - name: compile - run: | - cd build - make -j ${{env.proc_num}} - - clang-compile-with-bazel: - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - - run: bazel build --verbose_failures --action_env=CC=clang-12 -- //... -//example/... - - clang-compile-with-boringssl: - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - - run: bazel build --verbose_failures --action_env=CC=clang-12 --define with_mesalink=false --define with_glog=true --define with_thrift=true --define BRPC_WITH_BORINGSSL=true -- //... -//example/... - - clang-compile-with-make-all-options: - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - - uses: ./.github/actions/install-all-dependences - - uses: ./.github/actions/init-make-config - with: - options: --cc=clang --cxx=clang++ --with-thrift --with-glog --with-rdma --with-debug-bthread-sche-safety --with-debug-lock --with-bthread-tracer - - name: compile - run: | - make -j ${{env.proc_num}} - - clang-compile-with-cmake-all-options: - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - - uses: ./.github/actions/install-all-dependences - - name: cmake - run: | - export CC=clang && export CXX=clang++ - mkdir build - cd build - cmake -DWITH_MESALINK=OFF -DWITH_GLOG=ON -DWITH_THRIFT=ON -DWITH_RDMA=ON -DWITH_DEBUG_BTHREAD_SCHE_SAFETY=ON -DWITH_DEBUG_LOCK=ON -WITH_BTHREAD_TRACER=ON .. - - name: compile - run: | - cd build - make -j ${{env.proc_num}} - - clang-compile-with-bazel-all-options: - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - - run: bazel build --verbose_failures --action_env=CC=clang-12 --define with_mesalink=false --define with_glog=true --define with_thrift=true --define with_debug_bthread_sche_safety=true --define with_debug_lock=true -- //... -//example/... - clang-unittest: runs-on: ubuntu-20.04 steps: @@ -174,8 +26,173 @@ jobs: run: | cat config.mk cd test - make -j ${{env.proc_num}} + make -j ${{env.proc_num}} bthread_unittest - name: run tests run: | cd test - sh ./run_tests.sh + for i in `seq 1 10`; do sh ./run_tests.sh; done +# gcc-compile-with-make: +# runs-on: ubuntu-20.04 # https://github.com/actions/runner-images +# steps: +# - uses: actions/checkout@v2 +# - uses: ./.github/actions/install-essential-dependences +# - uses: ./.github/actions/init-make-config +# with: +# options: --cc=gcc --cxx=g++ +# - name: compile +# run: | +# make -j ${{env.proc_num}} +# +# gcc-compile-with-cmake: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - uses: ./.github/actions/install-essential-dependences +# - name: cmake +# run: | +# export CC=gcc && export CXX=g++ +# mkdir build +# cd build +# cmake .. +# - name: compile +# run: | +# cd build +# make -j ${{env.proc_num}} +# +# gcc-compile-with-bazel: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - run: bazel test --verbose_failures -- //... -//example/... +# +# gcc-compile-with-boringssl: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - run: bazel test --verbose_failures --define with_mesalink=false --define with_glog=true --define with_thrift=true --define BRPC_WITH_BORINGSSL=true -- //... -//example/... +# +# gcc-compile-with-make-all-options: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - uses: ./.github/actions/install-all-dependences +# - uses: ./.github/actions/init-make-config +# with: +# options: --cc=gcc --cxx=g++ --with-thrift --with-glog --with-rdma --with-debug-bthread-sche-safety --with-debug-lock --with-bthread-tracer +# - name: compile +# run: | +# make -j ${{env.proc_num}} +# +# gcc-compile-with-cmake-all-options: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - uses: ./.github/actions/install-all-dependences +# - name: cmake +# run: | +# export CC=gcc && export CXX=g++ +# mkdir build +# cd build +# cmake -DWITH_MESALINK=OFF -DWITH_GLOG=ON -DWITH_THRIFT=ON -DWITH_RDMA=ON -DWITH_DEBUG_BTHREAD_SCHE_SAFETY=ON -DWITH_DEBUG_LOCK=ON -WITH_BTHREAD_TRACER=ON .. +# - name: compile +# run: | +# cd build +# make -j ${{env.proc_num}} +# +# gcc-compile-with-bazel-all-options: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - run: bazel test --verbose_failures --define with_mesalink=false --define with_glog=true --define with_thrift=true --define with_debug_bthread_sche_safety=true --define with_debug_lock=true -- //... -//example/... +# +# clang-compile-with-make: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - uses: ./.github/actions/install-essential-dependences +# - uses: ./.github/actions/init-make-config +# with: +# options: --cc=clang --cxx=clang++ +# - name: compile +# run: | +# make -j ${{env.proc_num}} +# +# clang-compile-with-cmake: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - uses: ./.github/actions/install-essential-dependences +# - name: cmake +# run: | +# export CC=clang && export CXX=clang++ +# mkdir build +# cd build +# cmake .. +# - name: compile +# run: | +# cd build +# make -j ${{env.proc_num}} +# +# clang-compile-with-bazel: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - run: bazel build --verbose_failures --action_env=CC=clang-12 -- //... -//example/... +# +# clang-compile-with-boringssl: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - run: bazel build --verbose_failures --action_env=CC=clang-12 --define with_mesalink=false --define with_glog=true --define with_thrift=true --define BRPC_WITH_BORINGSSL=true -- //... -//example/... +# +# clang-compile-with-make-all-options: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - uses: ./.github/actions/install-all-dependences +# - uses: ./.github/actions/init-make-config +# with: +# options: --cc=clang --cxx=clang++ --with-thrift --with-glog --with-rdma --with-debug-bthread-sche-safety --with-debug-lock --with-bthread-tracer +# - name: compile +# run: | +# make -j ${{env.proc_num}} +# +# clang-compile-with-cmake-all-options: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - uses: ./.github/actions/install-all-dependences +# - name: cmake +# run: | +# export CC=clang && export CXX=clang++ +# mkdir build +# cd build +# cmake -DWITH_MESALINK=OFF -DWITH_GLOG=ON -DWITH_THRIFT=ON -DWITH_RDMA=ON -DWITH_DEBUG_BTHREAD_SCHE_SAFETY=ON -DWITH_DEBUG_LOCK=ON -WITH_BTHREAD_TRACER=ON .. +# - name: compile +# run: | +# cd build +# make -j ${{env.proc_num}} +# +# clang-compile-with-bazel-all-options: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - run: bazel build --verbose_failures --action_env=CC=clang-12 --define with_mesalink=false --define with_glog=true --define with_thrift=true --define with_debug_bthread_sche_safety=true --define with_debug_lock=true -- //... -//example/... +# +# clang-unittest: +# runs-on: ubuntu-20.04 +# steps: +# - uses: actions/checkout@v2 +# - uses: ./.github/actions/install-essential-dependences +# - uses: ./.github/actions/init-ut-make-config +# with: +# options: --cc=clang --cxx=clang++ --with-bthread-tracer +# - name: compile tests +# run: | +# cat config.mk +# cd test +# make -j ${{env.proc_num}} +# - name: run tests +# run: | +# cd test +# sh ./run_tests.sh diff --git a/.github/workflows/ci-macos.yml b/.github/workflows/ci-macos.yml deleted file mode 100644 index 50ab2e0294..0000000000 --- a/.github/workflows/ci-macos.yml +++ /dev/null @@ -1,109 +0,0 @@ -name: Build on Macos - -on: - push: - branches: [ master ] - paths-ignore: - - '**.md' - pull_request: - branches: [ master ] - paths-ignore: - - '**.md' - -env: - proc_num: $(sysctl -n hw.logicalcpu) - -jobs: - compile-with-make: - runs-on: macos-latest # https://github.com/actions/runner-images - - steps: - - uses: actions/checkout@v2 - - - name: install dependences - run: | - brew install ./homebrew-formula/protobuf.rb - brew install openssl gnu-getopt coreutils gflags leveldb - - - name: config_brpc - run: | - GETOPT_PATH=$(brew --prefix gnu-getopt)/bin - export PATH=$GETOPT_PATH:$PATH - ./config_brpc.sh --header="$(brew --prefix)/include" --libs="$(brew --prefix)/lib" - - - name: compile - run: | - make -j ${{env.proc_num}} - - compile-with-cmake: - runs-on: macos-latest - - steps: - - uses: actions/checkout@v2 - - - name: install dependences - run: | - brew install ./homebrew-formula/protobuf.rb - brew install openssl gflags leveldb - - - name: cmake - run: | - mkdir build - cd build - cmake .. - - - name: compile - run: | - cd build - make -j ${{env.proc_num}} - - compile-with-make-protobuf23: - runs-on: macos-latest # https://github.com/actions/runner-images - - steps: - - uses: actions/checkout@v2 - - - name: install dependences - run: | - brew install openssl gnu-getopt coreutils gflags leveldb - # abseil 20230125.3 - curl -o abseil.rb https://raw.githubusercontent.com/Homebrew/homebrew-core/b85b8dbf23ad509f163677a88ac72268f31e9c4a/Formula/abseil.rb - # protobuf 23.3 - curl -o protobuf.rb https://raw.githubusercontent.com/Homebrew/homebrew-core/b85b8dbf23ad509f163677a88ac72268f31e9c4a/Formula/protobuf.rb - HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK=1 brew install --formula --ignore-dependencies ./abseil.rb ./protobuf.rb - - - name: config_brpc - run: | - GETOPT_PATH=$(brew --prefix gnu-getopt)/bin - export PATH=$GETOPT_PATH:$PATH - ./config_brpc.sh --header="$(brew --prefix)/include" --libs="$(brew --prefix)/lib" - - - name: compile - run: | - make -j ${{env.proc_num}} - - compile-with-cmake-protobuf23: - runs-on: macos-latest - - steps: - - uses: actions/checkout@v2 - - - name: install dependences - run: | - brew install openssl gflags leveldb - # abseil 20230125.3 - curl -o abseil.rb https://raw.githubusercontent.com/Homebrew/homebrew-core/b85b8dbf23ad509f163677a88ac72268f31e9c4a/Formula/abseil.rb - # protobuf 23.3 - curl -o protobuf.rb https://raw.githubusercontent.com/Homebrew/homebrew-core/b85b8dbf23ad509f163677a88ac72268f31e9c4a/Formula/protobuf.rb - HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK=1 brew install --formula --ignore-dependencies ./abseil.rb ./protobuf.rb - - - name: cmake - run: | - mkdir build - cd build - cmake .. - - - name: compile - run: | - cd build - make -j ${{env.proc_num}} diff --git a/.github/workflows/cifuzz.yml b/.github/workflows/cifuzz.yml deleted file mode 100644 index 32178be1a3..0000000000 --- a/.github/workflows/cifuzz.yml +++ /dev/null @@ -1,38 +0,0 @@ -name: CIFuzz -on: - schedule: - - cron: '0 0 * * 0' -permissions: {} -jobs: - Fuzzing: - runs-on: ubuntu-latest - permissions: - security-events: write - steps: - - name: Build Fuzzers - id: build - uses: google/oss-fuzz/infra/cifuzz/actions/build_fuzzers@master - with: - oss-fuzz-project-name: 'brpc' - language: c++ - - name: Run Fuzzers - uses: google/oss-fuzz/infra/cifuzz/actions/run_fuzzers@master - with: - oss-fuzz-project-name: 'brpc' - language: c++ - fuzz-seconds: 300 - output-sarif: true - - name: Upload Crash - uses: actions/upload-artifact@v4 - if: failure() && steps.build.outcome == 'success' - with: - name: artifacts - path: ./out/artifacts - - name: Upload Sarif - if: always() && steps.build.outcome == 'success' - uses: github/codeql-action/upload-sarif@v2 - with: - # Path to SARIF file relative to the root of the repository - sarif_file: cifuzz-sarif/results.sarif - checkout_path: cifuzz-sarif - category: CIFuzz diff --git a/src/brpc/input_messenger.cpp b/src/brpc/input_messenger.cpp index d6e1c35670..b28c704415 100644 --- a/src/brpc/input_messenger.cpp +++ b/src/brpc/input_messenger.cpp @@ -287,8 +287,7 @@ int InputMessenger::ProcessNewMessage( // This unique_ptr prevents msg to be lost before transfering // ownership to last_msg DestroyingPtr msg(pr.message()); - QueueMessage(last_msg.release(), &num_bthread_created, - m->_keytable_pool); + QueueMessage(last_msg.release(), &num_bthread_created, m->_keytable_pool); if (_handlers[index].process == NULL) { LOG(ERROR) << "process of index=" << index << " is NULL"; continue; diff --git a/src/bthread/task_group.cpp b/src/bthread/task_group.cpp index 23333d4388..d8e1977d75 100644 --- a/src/bthread/task_group.cpp +++ b/src/bthread/task_group.cpp @@ -1013,13 +1013,15 @@ void print_task(std::ostream& os, bthread_t tid) { << "}\nhas_tls=" << has_tls << "\nuptime_ns=" << butil::cpuwide_time_ns() - cpuwide_start_ns << "\ncputime_ns=" << stat.cputime_ns - << "\nnswitch=" << stat.nswitch + << "\nnswitch=" << stat.nswitch #ifdef BRPC_BTHREAD_TRACER << "\nstatus=" << status << "\ntraced=" << traced << "\nworker_tid=" << worker_tid; -#endif // BRPC_BTHREAD_TRACER +#else ; + (void)status;(void)traced;(void)worker_tid; +#endif // BRPC_BTHREAD_TRACER } } diff --git a/src/bthread/task_tracer.cpp b/src/bthread/task_tracer.cpp index 2d8e509d6d..3b99b026d4 100644 --- a/src/bthread/task_tracer.cpp +++ b/src/bthread/task_tracer.cpp @@ -15,23 +15,46 @@ // specific language governing permissions and limitations // under the License. -#ifdef BRPC_BTHREAD_TRACER +// #ifdef BRPC_BTHREAD_TRACER #include "bthread/task_tracer.h" +#include +#include +#include +#include +#include "butil/fd_utility.h" #include "butil/debug/stack_trace.h" #include "butil/memory/scope_guard.h" #include "bthread/task_group.h" #include "bthread/processor.h" +// todo move到butil？ +#include "brpc/reloadable_flags.h" namespace bthread { DEFINE_bool(enable_fast_unwind, true, "Whether to enable fast unwind"); DEFINE_uint32(signal_trace_timeout_ms, 50, "Timeout for signal trace in ms"); +BRPC_VALIDATE_GFLAG(signal_trace_timeout_ms, [](const char*, uint32_t v) { + return v > 0; +}); extern BAIDU_THREAD_LOCAL TaskMeta* pthread_fake_meta; int TaskTracer::Init() { - if (RegisterSignalHandler() != 0) { + if (pipe(_pipe_fds) != 0) { + PLOG(ERROR) << "Fail to pipe"; + return -1; + } + if (butil::make_non_blocking(_pipe_fds[0]) != 0) { + PLOG(ERROR) << "Fail to make_non_blocking"; + return -1; + } + if (butil::make_non_blocking(_pipe_fds[1]) != 0) { + PLOG(ERROR) << "Fail to make_non_blocking"; + return -1; + } + if (sem_init(&_sem, 0, 0) != 0) { + PLOG(ERROR) << "Fail to sem_init"; return -1; } if (_trace_time.expose("bthread_trace_time") != 0) { @@ -43,6 +66,9 @@ int TaskTracer::Init() { if (_signal_handler_time.expose("bthread_signal_handler_time") != 0) { return -1; } + if (RegisterSignalHandler() != 0) { + return -1; + } return 0; } @@ -155,6 +181,7 @@ TaskTracer::Result TaskTracer::TraceImpl(bthread_t tid) { BRPC_SCOPE_EXIT { timer.stop(); _trace_time << timer.n_elapsed(); + LOG(INFO) << "Trace time: " << timer.u_elapsed(); }; if (tid == bthread_self() || @@ -277,7 +304,7 @@ int TaskTracer::RegisterSignalHandler() { return -1; } if (NULL != old_sa.sa_handler || NULL != old_sa.sa_sigaction) { - LOG(ERROR) << "SIGURG is already registered"; + LOG(ERROR) << "Signal handler of SIGURG is already registered"; return -1; } @@ -296,12 +323,6 @@ void TaskTracer::SignalHandler(int, siginfo_t* info, void* context) { // Caution: This function is called in signal handler, so it should be async-signal-safety. void TaskTracer::SignalTraceHandler(unw_context_t* context) { - // Something wrong, signal trace is not started, do nothing. - if (SIGNAL_TRACE_STATUS_START != - _signal_handler_flag.load(butil::memory_order_acquire)) { - return; - } - butil::Timer timer(butil::Timer::STARTED); BRPC_SCOPE_EXIT { timer.stop(); @@ -309,23 +330,114 @@ void TaskTracer::SignalTraceHandler(unw_context_t* context) { }; _signal_handler_context = context; - // Use memory_order_seq_cst to ensure the flag is set before loop. - _signal_handler_flag.store(SIGNAL_TRACE_STATUS_TRACING, butil::memory_order_seq_cst); - while (SIGNAL_TRACE_STATUS_TRACING == - _signal_handler_flag.load(butil::memory_order_seq_cst)) { - SignalSafeUsleep(50); // 50us - // Timeout to avoid deadlock of libunwind. + // Notify SignalTrace that SignalTraceHandler has started. + // Binary semaphore do not fail, so no need to check return value. + sem_post(&_sem); + pollfd poll_fd = {_pipe_fds[0], POLLIN, 0}; + while (true) { timer.stop(); - if (timer.m_elapsed() > FLAGS_signal_trace_timeout_ms) { + int timeout = FLAGS_signal_trace_timeout_ms - timer.m_elapsed(); + int rc = poll(&poll_fd, 1, timeout); + if (rc > 0) { + break; + } else if (rc == 0) { + errno = ETIMEDOUT; + return; + } else { + if (errno == EINTR) { + continue; + } + return; + } + } + + // Notify SignalTrace that SignalTraceHandler has started. + // Binary semaphore do not fail, so no need to check return value. + sem_post(&_sem); +} + +bool TaskTracer::ClearForSignalTrace(Result& result) { + // Clear the pipe. + while (true) { + char c; + ssize_t nr = read(_pipe_fds[1], &c, 1); + if (nr > 0) { + continue; + } else if (0 == nr) { + break; + } else { + if (EINTR == errno) { + continue; + } else if (EAGAIN == errno) { + break; + } + result.SetError("Fail to read pipe: %s", berror()); + return false; + } + } + + // Clear the semaphore. + while (true) { + int rc = sem_trywait(&_sem); + if (0 == rc) { + continue; + } else { + if (EINTR == errno) { + continue; + } else if (EAGAIN == errno) { + // The semaphore is already cleared. + break; + } + result.SetError("Fail to sem_trywait: %s", berror()); + return false; + } + } + + return true; +} + +bool TaskTracer::WaitForSignalHandler(const timespec* abs_timeout, Result& result) { + while (sem_timedwait(&_sem, abs_timeout) == -1) { + if (EINTR == errno) { + continue; + } + if (ETIMEDOUT == errno) { + result.SetError("Timeout exceed %dms", FLAGS_signal_trace_timeout_ms); + } else { + // During the process of signal handler, + // can not use berro() which is not async-signal-safe. + result.SetError("Fail to sem_timedwait, errno=%d", errno); + } + return false; + } + return true; +} + +bool TaskTracer::NotifySignalHandler(Result& result) { + while (true) { + ssize_t nw = write(_pipe_fds[1], "1", 1); + if (0 < nw) { break; + } else if (0 == nw) { + continue; + } else { + if (EINTR == errno) { + continue; + } else if (EAGAIN == errno) { + break; + } + result.SetError("Fail to write pipe: %s", berror()); + return false; } } + return true; } TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) { - _signal_handler_context = NULL; - // Use memory_order_seq_cst to ensure the flag is set before sending signal. - _signal_handler_flag.store(SIGNAL_TRACE_STATUS_START, butil::memory_order_seq_cst); + Result result; + if (!ClearForSignalTrace(result)) { + return result; + } // CAUTION: // The signal handler will wait for the backtrace to complete. @@ -345,8 +457,7 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) { // #0 __lll_lock_wait (futex=futex@entry=0x7f0d3d7f0990 <_rtld_global+2352>, private=0) at lowlevellock.c:52 // #1 0x00007f0d3a73c131 in __GI___pthread_mutex_lock (mutex=0x7f0d3d7f0990 <_rtld_global+2352>) at ../nptl/pthread_mutex_lock.c:115 // #2 0x00007f0d38eb0231 in __GI___dl_iterate_phdr (callback=callback@entry=0x7f0d38c456a0 <_ULx86_64_dwarf_callback>, data=data@entry=0x7f0d07defad0) at dl-iteratephdr.c:40 - // #3 0x00007f0d38c45d79 in _ULx86_64_dwarf_find_proc_info (as=0x7f0d38c4f340 , ip=ip@entry=139694791966897, pi=pi@entry=0x7f0d07df0498, need_unwind_info=need_unwind_info@entry=1, arg=0x7f0 - // d07df0340) at dwarf/Gfind_proc_info-lsb.c:759 + // #3 0x00007f0d38c45d79 in _ULx86_64_dwarf_find_proc_info (as=0x7f0d38c4f340 , ip=ip@entry=139694791966897, pi=pi@entry=0x7f0d07df0498, need_unwind_info=need_unwind_info@entry=1, arg=0x7f0d07df0340) at dwarf/Gfind_proc_info-lsb.c:759 // #4 0x00007f0d38c43260 in fetch_proc_info (c=c@entry=0x7f0d07df0340, ip=139694791966897) at dwarf/Gparser.c:461 // #5 0x00007f0d38c44e46 in find_reg_state (sr=0x7f0d07defd10, c=0x7f0d07df0340) at dwarf/Gparser.c:925 // #6 _ULx86_64_dwarf_step (c=c@entry=0x7f0d07df0340) at dwarf/Gparser.c:972 @@ -374,6 +485,10 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) { // backtracks with dl_iterate_phdr. We introduce a timeout mechanism in signal // handler to avoid deadlock. + butil::Timer timer(butil::Timer::STARTED); + + _signal_handler_context = NULL; + union sigval value{}; value.sival_ptr = this; size_t sigqueue_try = 0; @@ -383,36 +498,30 @@ TaskTracer::Result TaskTracer::SignalTrace(pid_t tid) { } } - butil::Timer timer(butil::Timer::STARTED); - // Use memory_order_seq_cst to ensure the signal is sent and the flag is set before checking. - for (int i = 0; - SIGNAL_TRACE_STATUS_START == _signal_handler_flag.load(butil::memory_order_seq_cst); - ++i) { - if (i < 30) { - sched_yield(); - } else { - SignalSafeUsleep(5); // 5us - } - - // Timeout to avoid dead loop if handler of SIGURG is covered. - timer.stop(); - if (timer.m_elapsed() > FLAGS_signal_trace_timeout_ms) { - return Result::MakeErrorResult( - "Timeout exceed %dms", FLAGS_signal_trace_timeout_ms); - } + timespec abs_timeout = butil::milliseconds_from_now(FLAGS_signal_trace_timeout_ms); + // Wait for the signal handler to start. + if (!WaitForSignalHandler(&abs_timeout, result)) { + return result; } unw_cursor_t cursor; - int rc = unw_init_local(&cursor, _signal_handler_context); - Result result; - if (0 == rc) { - result = TraceCore(cursor); + int rc = unw_init_local(&cursor, _signal_handler_context); + if (0 != rc) { + return Result::MakeErrorResult("Failed to init local, rc=%d", rc); } + result = TraceCore(cursor); - // Use memory_order_seq_cst to ensure the flag is set after tracing. - _signal_handler_flag.store(SIGNAL_TRACE_STATUS_UNKNOWN, butil::memory_order_seq_cst); + NotifySignalHandler(result); - return 0 == rc ? result : Result::MakeErrorResult("Failed to init local, rc=%d", rc); + // Wait for the signal handler to end. + if (!WaitForSignalHandler(NULL, result)) { + return result; + } + + timer.stop(); + LOG(INFO) << "Signal trace time: " << timer.u_elapsed(); + + return result; } unw_cursor_t TaskTracer::MakeCursor(bthread_fcontext_t fcontext) { @@ -484,4 +593,4 @@ TaskTracer::Result TaskTracer::TraceCore(unw_cursor_t& cursor) { } // namespace bthread -#endif // BRPC_BTHREAD_TRACER +// #endif // BRPC_BTHREAD_TRACER diff --git a/src/bthread/task_tracer.h b/src/bthread/task_tracer.h index 2f332f5a31..aebe0aa317 100644 --- a/src/bthread/task_tracer.h +++ b/src/bthread/task_tracer.h @@ -18,9 +18,9 @@ #ifndef BRPC_BTHREAD_TRACER_H #define BRPC_BTHREAD_TRACER_H -#ifdef BRPC_BTHREAD_TRACER +// #ifdef BRPC_BTHREAD_TRACER -#include +#include #include #include #include "butil/strings/safe_sprintf.h" @@ -60,12 +60,6 @@ class TaskTracer { int _errno; }; - enum SignalTraceStatus { - SIGNAL_TRACE_STATUS_UNKNOWN = 0, - SIGNAL_TRACE_STATUS_START, - SIGNAL_TRACE_STATUS_TRACING, - }; - struct Result { template static Result MakeErrorResult(const char* fmt, Args... args) { @@ -75,14 +69,18 @@ class TaskTracer { return result; } + template + Result SetError(const char* fmt, Args... args) { + error = true; + butil::strings::SafeSPrintf(err_msg, fmt, args...); + } + static const size_t MAX_TRACE_NUM = 64; unw_word_t ips[MAX_TRACE_NUM]; + char mangled[MAX_TRACE_NUM][256]{}; size_t frame_count{0}; bool error{false}; - union { - char mangled[MAX_TRACE_NUM][256]{}; - char err_msg[256]; - }; + char err_msg[128]{}; bool fast_unwind{false}; }; @@ -97,6 +95,9 @@ class TaskTracer { static int RegisterSignalHandler(); static void SignalHandler(int sig, siginfo_t* info, void* context); void SignalTraceHandler(unw_context_t* context); + bool ClearForSignalTrace(Result& result); + bool WaitForSignalHandler(const timespec* abs_timeout, Result& result); + bool NotifySignalHandler(Result& result); Result SignalTrace(pid_t worker_tid); unw_cursor_t MakeCursor(bthread_fcontext_t fcontext); @@ -114,7 +115,9 @@ class TaskTracer { unw_context_t _context{}; // For signal trace. unw_context_t* _signal_handler_context{NULL}; - butil::atomic _signal_handler_flag{SIGNAL_TRACE_STATUS_UNKNOWN}; + sem_t _sem{}; + int _pipe_fds[2]{}; + // butil::atomic _signal_handler_flag{SIGNAL_TRACE_STATUS_UNKNOWN}; // Protect `_worker_tids'. butil::Mutex _worker_mutex; @@ -129,4 +132,4 @@ class TaskTracer { #endif // BRPC_BTHREAD_TRACER -#endif // BRPC_BTHREAD_TRACER_H +// #endif // BRPC_BTHREAD_TRACER_H diff --git a/test/bthread_unittest.cpp b/test/bthread_unittest.cpp index 0283875eb6..5484cd4655 100644 --- a/test/bthread_unittest.cpp +++ b/test/bthread_unittest.cpp @@ -617,11 +617,144 @@ TEST_F(BthreadTest, yield_single_thread) { ASSERT_EQ(0, bthread_join(tid, NULL)); } -#ifdef BRPC_BTHREAD_TRACER +typedef void* (test_fn_t)(void*); + +void* test(void* arg) { + std::unique_ptr p(new int); + auto fn = (test_fn_t*)arg; + return (*fn)((void*)1); +} + +void* test1(void* arg) { + std::unique_ptr p(new int); + return test(arg); +} + +void* test2(void* arg) { + std::unique_ptr p(new int); + return test1(arg); +} + +void* test3(void* arg) { + std::unique_ptr p(new int); + return test2(arg); +} + +void* test4(void* arg) { + std::unique_ptr p(new int); + return test3(arg); +} + +void* test5(void* arg) { + std::unique_ptr p(new int); + return test4(arg); +} + +void* test6(void* arg) { + std::unique_ptr p(new int); + return test5(arg); +} + +void* test7(void* arg) { + std::unique_ptr p(new int); + return test6(arg); +} + +void* test8(void* arg) { + std::unique_ptr p(new int); + return test7(arg); +} + +void* test9(void* arg) { + std::unique_ptr p(new int); + return test8(arg); +} + +void* test10(void* arg) { + std::unique_ptr p(new int); + return test9(arg); +} + +void* test11(void* arg) { + std::unique_ptr p(new int); + return test10(arg); +} + +void* test12(void* arg) { + std::unique_ptr p(new int); + return test11(arg); +} + +void* test13(void* arg) { + std::unique_ptr p(new int); + return test12(arg); +} + +void* test14(void* arg) { + std::unique_ptr p(new int); + return test13(arg); +} + +void* test15(void* arg) { + std::unique_ptr p(new int); + return test14(arg); +} + +void* test16(void* arg) { + std::unique_ptr p(new int); + return test15(arg); +} + +void* test17(void* arg) { + std::unique_ptr p(new int); + return test16(arg); +} + +void* test18(void* arg) { + std::unique_ptr p(new int); + return test17(arg); +} + +void* test19(void* arg) { + std::unique_ptr p(new int); + return test18(arg); +} + +void* test20(void* arg) { + std::unique_ptr p(new int); + return test19(arg); +} + +void* test21(void* arg) { + std::unique_ptr p(new int); + return test20(arg); +} + +void* test22(void* arg) { + std::unique_ptr p(new int); + return test21(arg); +} + +void* test23(void* arg) { + std::unique_ptr p(new int); + return test22(arg); +} + +void* test24(void* arg) { + std::unique_ptr p(new int); + return test23(arg); +} + +void* test25(void* arg) { + std::unique_ptr p(new int); + return test24(arg); +} + +// #ifdef BRPC_BTHREAD_TRACER TEST_F(BthreadTest, trace) { stop = false; bthread_t th; - ASSERT_EQ(0, bthread_start_urgent(&th, NULL, spin_and_log, (void*)1)); + ASSERT_EQ(0, bthread_start_urgent(&th, NULL, test25, (void*)spin_and_log)); usleep(100 * 1000); std::string st = bthread::stack_trace(th); LOG(INFO) << "spin_and_log stack trace:\n" << st; @@ -630,7 +763,7 @@ TEST_F(BthreadTest, trace) { ASSERT_EQ(0, bthread_join(th, NULL)); stop = false; - ASSERT_EQ(0, bthread_start_urgent(&th, NULL, repeated_sleep, (void*)1)); + ASSERT_EQ(0, bthread_start_urgent(&th, NULL, test25, (void*)repeated_sleep)); usleep(100 * 1000); st = bthread::stack_trace(th); LOG(INFO) << "repeated_sleep stack trace:\n" << st; @@ -643,6 +776,6 @@ TEST_F(BthreadTest, trace) { ASSERT_NE(std::string::npos, st.find("not exist now")); } -#endif // BRPC_BTHREAD_TRACER +// #endif // BRPC_BTHREAD_TRACER } // namespace diff --git a/test/run_tests.sh b/test/run_tests.sh index 142977975a..dd220b5c1d 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -22,11 +22,11 @@ rm core.* test_num=0 failed_test="" rc=0 -test_bins="test_butil test_bvar bthread*unittest brpc*unittest" +test_bins="bthread_unittest" for test_bin in $test_bins; do test_num=$((test_num + 1)) >&2 echo "[runtest] $test_bin" - ./$test_bin + ./$test_bin --gtest_filter=BthreadTest.trace rc=$? if [ $rc -ne 0 ]; then failed_test="$test_bin"