Repository: llvm/llvm-project
File: clang-tools-extra/clangd/unittests/ThreadCrashReporterTests.cpp (C++)

Code before:
///===- ThreadCrashReporterTests.cpp - Thread local signal handling tests -===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "support/ThreadCrashReporter.h"
#include "support/Threading.h"
#include "llvm/Support/Signals.h"
#include "gtest/gtest.h"
#include <csignal>
#include <string>

// According to the POSIX specification, if the inherited disposition of a
// signal is the default action, the behavior of utilitys must be as if the
// default action had been taken. When the required default action is to
// terminate the process, such as for SIGUSR1, the utility may catch the
// signal, perform additional processing, restore the default disposition,
// and then re-signal itself. This causes the process to terminate as
// required. Because of this behavior, the crash-reporter test here is not
// suitable for Unix platforms.

#ifndef LLVM_ON_UNIX

namespace clang {
namespace clangd {

namespace {

static void infoSignalHandler() { ThreadCrashReporter::runCrashHandlers(); }

TEST(ThreadCrashReporterTest, All) {
#if defined(_WIN32)
  // Simulate signals on Windows for unit testing purposes.
  // The `crash.test` lit test checks the end-to-end integration.
  auto SignalCurrentThread = []() { infoSignalHandler(); };
#else
  llvm::sys::SetInfoSignalFunction(&infoSignalHandler);
  auto SignalCurrentThread = []() { raise(SIGUSR1); };

Review comment (suggestion):
IMHO this is a too broad interpretation of the POSIX utilities clause. IIUC said clause only applies to utilities named via the specification (i.e. `clang` when invoked as `c17` for example)

For utilities such as `clangd` which fall well outside POSIX, they are perfectly entitled to have a different signal handling behaviour, and it would be good to find a way to let them preserve their customized behaviour in this patch.

Code after:
///===- ThreadCrashReporterTests.cpp - Thread local signal handling tests -===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "support/ThreadCrashReporter.h"
#include "support/Threading.h"
#include "llvm/Support/Signals.h"
#include "gtest/gtest.h"
#include <csignal>
#include <string>

namespace clang {
namespace clangd {

namespace {

static void infoSignalHandler() { ThreadCrashReporter::runCrashHandlers(); }

TEST(ThreadCrashReporterTest, All) {
#if defined(_WIN32)
  // Simulate signals on Windows for unit testing purposes.
  // The `crash.test` lit test checks the end-to-end integration.
  auto SignalCurrentThread = []() { infoSignalHandler(); };
#else
  llvm::sys::SetInfoSignalFunction(&infoSignalHandler);
  auto SignalCurrentThread = []() { raise(SIGUSR1); };
#endif

  AsyncTaskRunner Runner;
  auto SignalAnotherThread = [&]() {
    Runner.runAsync("signal another thread", SignalCurrentThread);
    Runner.wait();
  };

  bool Called;
  {
    ThreadCrashReporter ScopedReporter([&Called]() { Called = true; });