-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Add note to C.30 (or new CP rule) for explicit destructors in classes with concurrency dependencies to prevent UB from member destruction order #2316
Description
C.30 (Define a destructor if a class needs an explicit action at object destruction)provides excellent guidance on when to define a user-declared destructor, focusing on resource management and scope-guard actions. The notes highlight two general categories:
- A class with a resource not already represented as a class with a destructor (e.g., vector or a transaction class).
- A class that exists primarily to execute an action upon destruction (e.g., a tracer or final_action).
However, this assumes a single-threaded context where member destruction order is predictable and safe. In concurrent code, where a class spawns or manages threads/futures that access other members, the default destructor can lead to undefined behavior (UB) due to the reverse declaration order of destruction.
Threads may outlive or access members that have already been destroyed, causing crashes, data races, or silent corruption.This pitfall is not currently addressed in C.30 or the CP (Concurrency and parallelism) section. It is a hidden, high-severity issue in production code:
- hard to reproduce (timing-dependent),
- often missed in code reviews (as the code is "standard-compliant")
- disruptive to testing workflows.
Consider this minimal example using std::async (which spawns a thread). The class has no explicit destructor, relying on the default one:
#include <iostream>
#include <future>
#include <thread>
#include <memory>
#include <chrono>
// Define a class spinning up a thread
class FooClass {
public:
FooClass()
: value_(std::make_unique<int>(12))
{
// Spin up a separate thread
future_ = std::async([this]() {
// Wait for 1 second before executing any actions in the thread
std::this_thread::sleep_for(std::chrono::seconds(1));
std::cout << "Starting a separate thread!" << std::endl;
std::cout << "Value: " << *value_ << std::endl;
});
}
std::future<void> future_;
std::unique_ptr<int> value_;
};
int main() {
FooClass obj;
// Object goes out of scope here; default destructor runs
}Here is a output.
rainy-juzixiao@DEKSTOP-LAM5KS:/media/data/projects/test$ ./cmake-build-debug/test
Starting a separate thread!
/usr/include/c++/15/bits/unique_ptr.h:455: constexpr typename std::add_lvalue_reference<_Tp>::type std::unique_ptr<_Tp, _Dp>::operator*() const [with _Tp = int; _Dp = std::default_delete<int>; typename std::add_lvalue_reference<_Tp>::type = int&]: Assertion 'get() != pointer()' failed.
Root Cause:
- Members are destroyed in reverse declaration order:value_ is destroyed first.
future_ is destroyed next, but the async thread is still running and accesses the already-destroyed value_, triggering UB (dereferencing a null/deleted pointer). - std::thread itself enforces join()/detach() in its destructor to prevent termination, but this doesn't cover cases where threads access other members. The issue is amplified in custom thread wrappers or pools, where races are scheduler-dependent and hard to repro (e.g., only under high load).
I just want to say that the code compiles cleanly, but produces UB. Code reviews often overlook it because "default destructor is fine if members clean up."
Many devs implicitly add destructors in concurrent classes, but it's not a "=default" shortcut—it's necessary for safety. Without guidance, it's a trap for newcomers or rushed code.
I think we can add a third category to the C.30 notes:A class with concurrency dependencies, where threads or tasks access members and require explicit shutdown (e.g., joining threads before destroying shared resources).
Pin a note to C.30: "In classes managing concurrency (e.g., spawning threads/futures that access members), declare thread-related members before dependent resources to ensure reverse destruction order joins threads first. Consider an explicit destructor for custom shutdown logic."
Or, add a small rule in CP (e.g., CP.x: "Define explicit destructors for concurrent classes to enforce safe shutdown and respect member destruction order").
Tie it to RAII: Ensure destructors handle concurrency safely (e.g., join before destroying mutexes/queues).
This aligns with CP.26: Don’t detach() a thread, emphasizing RAII for threads.
Enforcement
- classes with std::thread, std::jthread, std::future, or custom threads without explicit destructors or proper member order.
- Static analyzers should warn on classes owning std::thread/jthread/future that use =default destructor without explicit shutdown logic, or where thread members are declared after accessed members.
Static analyzers could check declaration order against access patterns.
Thank you for reading this issues.