4
struct X
{
    std::mutex m;
    std::string str;
    
    void set(std::string s) 
    {
        auto _ = std::unique_lock(m);
        str = std::move(s);
    }
    
    ~X()
    {
        // auto _ = std::unique_lock(m);
    }
}

Is there any part of the standard that guarantees that ~X will never experience race conditions inside ~string without the commented line?

The exclusive access to the object and/or lifetime could be managed by an atomic variable with RELAXED semantics (== no other data except the fact of the end of lifetime is synched). We have a mutex to protect the access to the object, so using relaxed operation seems to be ok to synchronize exclusive access/lifetime and mutex to access the data.

If the standard would require ~mutex to synchronize with the latest unlock and if we moved the declaration of the mutex below the protected data then we could afford default ~X but without this requirement, we need always have explicit destructor which locks all the mutexes used to protect any member.

PS to help understand my question use this example https://en.cppreference.com/w/cpp/thread/mutex there is a comment that accessing g_pages without a lock is safe. Why, which part of the standard guarantees this? The fact that both threads are joined only guarantees no multi-threaded access to the map, but it doesn't guarantee synchronize-with relationship with the latest mutex::unlock operation. I run many different programs trying to expose race conditions and I am pretty sure that mutex uses std::atomic_thread_fence for both lock and unlock which synchronizes with thread.join that must use at least some atomic to work. But the problem is standard doesn't require mutex to use std::atomic_thread_fence and it it just happens that all known to me implementations use it.

#include <chrono>
#include <iostream>
#include <map>
#include <mutex>
#include <string>
#include <thread>
 
std::map<std::string, std::string> g_pages;
std::mutex g_pages_mutex;
 
void save_page(const std::string &url)
{
    // simulate a long page fetch
    std::this_thread::sleep_for(std::chrono::seconds(2));
    std::string result = "fake content";
 
    std::lock_guard<std::mutex> guard(g_pages_mutex);
    g_pages[url] = result;
}
 
int main() 
{
    std::thread t1(save_page, "http://foo");
    std::thread t2(save_page, "http://bar");
    t1.join();
    t2.join();
 
    // safe to access g_pages without lock now, as the threads are joined
    for (const auto &pair : g_pages)
        std::cout << pair.first << " => " << pair.second << '\n';
}

PPS as was pointed out by @user17732522 the example above is guaranteed to be safe because of thread.join. You can remove mutex and one of the threads (just use 1 thread) and it will be safe. So I modified this example slightly to demonstrate the problem. Note that relaxed memory order is important here. If we replace it with a pair of acquire/release it will be perfectly thread-safe code, but it is also correct with relaxed if we assume std::mutex::unlock uses std::atomic_thread_fence

#include <atomic>
#include <chrono>
#include <iostream>
#include <map>
#include <mutex>
#include <string>
#include <thread>

std::map<std::string, std::string> g_pages;
std::mutex g_pages_mutex;
std::atomic_flag g_f1 = {};
std::atomic_flag g_f2 = {};

void save_page(const std::string& url, std::atomic_flag* flag)
{
    // simulate a long page fetch
    std::this_thread::sleep_for(std::chrono::seconds(2));
    std::string result = "fake content";

    {
        std::lock_guard<std::mutex> guard(g_pages_mutex);
        g_pages[url] = result;
    }
    flag->clear(std::memory_order_relaxed);
}

int main()
{
    g_f1.test_and_set();
    g_f2.test_and_set();

    std::thread t1(save_page, "http://foo", &g_f1);
    std::thread t2(save_page, "http://bar", &g_f2);

    while (g_f1.test_and_set(std::memory_order_relaxed));
    while (g_f2.test_and_set(std::memory_order_relaxed));

    // whether it safe to access g_pages without lock now, as the threads signaled they are done?
    for (const auto& pair : g_pages)
        std::cout << pair.first << " => " << pair.second << '\n';

    t1.join();
    t2.join();
}
8
  • 6
    If a mutex is locked, it is UB to attempt to destroy it. Even if it wasn't, it is unclear how it is possible to have one thread to have any access to an object while the other thread is destroying it without having all sorts of problems. Assume that the commented line is uncommented—what would happen if the line is executed and another thread tries to acquire the lock immediately after that? Commented Oct 16, 2023 at 5:55
  • My question is about NO UB case. There is only 1 thread accessing x when ~X is executed, see my other comment under bitmask answer for details Commented Oct 16, 2023 at 20:49
  • Interesting. No, I don't think the standard guarantees that, it would be totally unreasonable. Commented Oct 16, 2023 at 21:08
  • 1
    The last example works because join synchronizes with the completion of the thread and everything else in the thread is sequenced-before completion. The mutex is not relevant at that point. Commented Oct 16, 2023 at 21:47
  • For the rest of the question: Synchronizing in the destructor is too late. The lifetime of the object ends with invocation of the destructor. So at that point already no other thread is allowed to access any member of the object anymore. Establishing a happens-before relationship through synchronizes-with inside the destructor is then already too late. Also the members are in the wrong order for this to work. Commented Oct 16, 2023 at 21:51

1 Answer 1

9

Sort of. The standard does require you not to destroy an object who's lifetime has ended. If you run into the situation where multiple threads are trying to run X::~X() on the same object, you are already in UB land.

A similar argument can be made for one thread trying to ~X some x and another trying to x.set on the same object. If you ever run into this situation you are already violating object access rules for x. The UB stemming from races on x.str are a consequence of this.

So, you should never find yourself in the situation that you need a lock in the destructor of your owning X to begin with. If several threads are accessing the same x object, have it maybe be owned outside the thread pool.

Sign up to request clarification or add additional context in comments.

16 Comments

yes, sure, what you say is correct, but my question was about another problem. The case I am worried about is when ~X is guaranteed to be called when no other threads are using x, but when some other thread (not that which calls ~X) was previously called set. That means the ~string might see an outdated internal pointer -> double free. Which part of standard guarantees that in this case (no multithreaded access, but some mutations were made previously in (an)other threads) destructor of ~X will see all the stores made in other threads without locking the mutex. This is EXTREMELY common pattern
@AntonDyachenko the definition of "happens before" does not include the case where one thread writes a value to an atomic variable and another thread reads the written value---UNLESS the write was a release write and the read was an acquire read.
Also, note that this: ~X { auto _ = std::unique_lock(m); } doesn't do anything useful at all. The order of events is that _ will be constructed, then destroyed (thereby releasing the lock), then X's members are destroyed, no longer being protected by the lock.
@AntonDyachenko A call to std::atomic_thread_fence in the writing thread doesn't actually establish a synchronization relationship unless there's an acquire operation in the reading thread (the acquire operation can be either another fence, or an atomic operation); see [atomics.fences] in the standard. I am not surprised that there is much code out in the real world that is not correct according to the standard, but happens to work anyway.
@AntonDyachenko You're wrong. Your lock dies before you're done destroying the object.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.