Crash when calling `std::exit` while server running or client requests in flight

[Y--]

Hi!

Thanks for the great library!

I think there is a race condition when the process is terminated explicitly (for example with std::exit) while a server is processing queries and/or a client is executing some.

I crafted a small reproduction below (note that in the actual code we're not "sleeping" in the atexit but have the same issue nonetheless):

#include "httplib.h"

struct ServerWrapper {
  httplib::Server server;
  std::unique_ptr<std::thread> s_thread;
};

static std::unique_ptr<ServerWrapper> s_wrapper;

void mainServer() {
  s_wrapper->server.Get("/hi", [](const httplib::Request &req, httplib::Response &res) {
    res.set_content("Quack", "text/plain");
  });

  s_wrapper->server.listen("localhost", 8080);
}

void exitThread() {
  std::cout << "Will exit soon..." << std::endl;
  std::this_thread::sleep_for(std::chrono::milliseconds(500));
  std::cout << "Calling std::exit now." << std::endl;
  std::exit(0);
}

void runClient() {
  httplib::Client cli("localhost", 8080);
  while (true) {
    auto res = cli.Get("/hi");
    std::this_thread::sleep_for(std::chrono::milliseconds(1));
  }
}

void atex() {
  std::cout << "Entering std::atexit callback, will wait a bit and stop server" << std::endl;
  // Wait a bit before stopping server to simulate delayed exit
  std::this_thread::sleep_for(std::chrono::milliseconds(500));
  std::cout << "[std::atexit] Waited, will stop server now." << std::endl;
  s_wrapper->server.stop();
  std::cout << "[std::atexit] Server stopped." << std::endl;
  s_wrapper->s_thread->join();
  std::cout << "[std::atexit] Server thread completed." << std::endl;
}

int main() {
  std::atexit(atex);
  s_wrapper = std::make_unique<ServerWrapper>();
  s_wrapper->s_thread = std::make_unique<std::thread>(&mainServer);

  std::thread cliThread(&runClient);

  std::thread(&exitThread).join();
  return 0;
}

Can be compiled with:

$ g++ -g -O0 -std=c++20 test.cc

Then execution looks like:

$  ./a.out
Will exit soon...
Calling std::exit now.
Entering std::atexit callback, will wait a bit and stop server
[1]    37193 segmentation fault  ./a.out

(MacOS 15.3.1; g++: Apple clang version 16.0.0 (clang-1600.0.26.6))

The issue (if I understand it correctly) is that the static destructors are executed before the server is actually stopped. So it is possible for example that a server requests arrives after the static methods object is destroyed, but still attempts to use it.

An easy fix would be to remove all static initializations (for example removing both this one and that one "fixes" the example above, but of course we could craft a different one to make it crash with another static resource).
But that is probably not acceptable for performance reasons.

The C++ standard guarantees that functions registered with std::atexit are called in the reverse order of their registration, except that a function is called after any previously registered functions that had already been called at the time it was registered.
As a consumer of the library, this makes it quite cumbersome to make sure that we define the atexit handler after each static initialization (to make sure it is called first, and has a chance to destroy the server before other static destructors)

Another way would be to hoist all static definition at the top of the file, so that caller of std::atexit would be guaranteed to have their handler run first.

Of course there are more involved way to fix the problem.

I am happy to contribute to a fix when we settle on the right approach :-)

[falbrechtskirchinger]

So, I looked into the use of static in this library a while ago, and I expected someone to have complained about exit-time destructors, but searching the issues, it appears no one has. I did not submit a PR for this, as the typical solution to heap allocate (see Chromium) makes using valgrind more difficult. Also, there are other problems; for example, random_string()s use of the random number generator isn't thread-safe (should be static thread_local).
All this to say that it's on my radar. I'd like to make it configurable and default to not using exit-time destructors.

[yhirose]

@Y-- thanks for the detailed problem report. I am trying to fully understand what's going on. Is this simpler code the same as what you reported?

#include <cstdlib>
#include <iostream>
#include <thread>

struct Variable {
  ~Variable() {
    std::cout << "`Variable` destructor" << std::endl;
    alive_ = false;
  }

  void method() {
    std::cout << "`Variable` is "
              << (alive_ ? "alive." : "NOT alive any more...") << std::endl;
  }

  bool alive_ = true;
};

struct Server {
  ~Server() { std::cout << "`Server` destructor" << std::endl; }

  void method() {
    static Variable local_static_variable;
    local_static_variable.method();
  }
};

struct Local {
  ~Local() { std::cout << "Local destructor will *not* be called\n"; }
};

Server server;

void runClient() {
  while (true) {
    server.method();
    std::this_thread::sleep_for(std::chrono::milliseconds(100));
  }
}

void exitThread() {
  std::cout << "# Will exit soon..." << std::endl;
  std::this_thread::sleep_for(std::chrono::milliseconds(500));
  std::cout << "# Calling std::exit now." << std::endl;
  std::exit(0);
}

void atexit_handler() {
  std::cout << "# Enter `atexit` handler" << std::endl;
  std::this_thread::sleep_for(std::chrono::milliseconds(500));
  std::cout << "# Leave `atexit` handler" << std::endl;
}

int main() {
  Local local;
  std::atexit(atexit_handler);
  std::thread cliThread(&runClient);
  std::thread(&exitThread).join();
  std::cout << "this line will *not* be executed" << std::endl;
}

The result is:

> g++ -std=c++11 exit_test.cpp && ./a.out
# Will exit soon...
`Variable` is alive.
`Variable` is alive.
`Variable` is alive.
`Variable` is alive.
`Variable` is alive.
# Calling std::exit now.
`Variable` destructor
# Enter `atexit` handler
`Variable` is NOT alive any more...
`Variable` is NOT alive any more...
`Variable` is NOT alive any more...
`Variable` is NOT alive any more...
`Variable` is NOT alive any more...
# Leave `atexit` handler
`Server` destructor

[Y--]

Hi @yhirose, yes this example clearly shows the problem as well indeed.
Basically we need to make sure that no Variable::method is ever called after ~Variable is.

[falbrechtskirchinger]

@Y-- Initial draft PR is up: #2105. Compiled with CPPHTTPLIB_NO_EXIT_TIME_DESTRUCTORS defined, your MRE should no longer crash, as the statics won't be destroyed before the atexit handler runs. (Untested for now.)

Edit: I have tested the code and am in the process of finalizing the PR. The published code has some errors that I've fixed.
Edit 2: Final/fixed code is now published.

[yhirose]

@Y-- could you try to use std::quick_exit and std::at_quick_exit to see if they can fix the segmentation fault problem? (The only thing with this approach is that destructors of global objects won't be executed.

Another way is to replace all the local static variables with static thread_local in httplib.h. In this case, destructors of global objects will be executed.

@falbrechtskirchinger thanks for the pull request #2105, but I actually prefer a simpler way over introducing the special macro. Personally, I like to use static thread_local unless there is any obvious reason that we can't use it. (Of course, all the contributors need to know this fact when submitting pull requests. So it should be documented in README.)

[falbrechtskirchinger]

Another way is to replace all the local static variables with static thread_local in httplib.h. In this case, destructors of global objects will be executed.

@falbrechtskirchinger thanks for the pull request #2105, but I actually prefer a simpler way over introducing the special macro. Personally, I like to use static thread_local unless there is any obvious reason that we can't use it.

Not the biggest fan of either solution, to be honest. I don't like the macro with the heap allocation; thread_local isn't exactly efficient, and the magnitude of inefficiency depends on the library user and their thread management. thread_local addresses this issue and allows httplib.h to be compiled with -Wexit-time-destructors on almost all platforms (not on Windows because of WSInit).

There's also the (obscure) phoenix design pattern, but I don't recommend that as a solution.

Personally, I would still go with the macro because I prefer leaking memory as the application exits over duplicating static variable instances for every thread (potentially many short-lived ones). Also, it enables singletons in the future, for which I see some potential use cases (async DNS, for example). Lastly, httplib.h can compile with -Wexit-time-destructors on all platforms, which I'm very surprised no one has complained about yet.

(Of course, all the contributors need to know this fact when submitting pull requests. So it should be documented in README.)

Time to add CONTRIBUTING.md?

[yhirose]

@Y--, @falbrechtskirchinger, the following is from Copilot about usage of std::exit. This issue is caused by the second point "Static Destructors".

After reading the answer, I am now considering making no adjustments in cpp-httplib for this issue. However, I would like you to try one of the following to prevent the crash:

1. Call s_wrapper->server.stop(); and s_wrapper->s_thread->join(); before calling std::exit(0); (The 3rd point "Cleanup Functions" under "Alternatives" section.)
2. Use std::quick_exit and std::at_quick_exit instead of std::exit and std::atexit. (Please see my previous comment.)

I think this issue is not a very common use case, as this is the first time I have received this kind of problem report. 😄 If this issue can be easily fixed on the user's side, I would like to keep the current httplib.h as it is. Of course, I'll update the README in case others experience the same issue in the future.

Thanks for your understanding!

[falbrechtskirchinger]

After reading the answer, I am now considering making no adjustments in cpp-httplib for this issue.

Agreed.