fix crashes during destruction of static objects

[the-be-to]

Fixes #7.

this_thread is defined here with thread storage duration

rpp/rpp/profile.h

Line 163 in 29ad6ef

static inline thread_local Thread_Profile this_thread;

and used here, in a place ultimately reached by the destructor with the default allocator (Mallocator with Log == true).

rpp/rpp/impl/profile.cpp

Lines 193 to 196 in 29ad6ef

	Thread::Lock lock(this_thread.frames_lock);
	if(this_thread.during_frame) {
	this_thread.frames.back().allocations.push(rpp::move(a));
	}

From [basic.start.term],

2. Constructed objects with thread storage duration within a given thread are destroyed as a result of returning from the initial function of that thread and as a result of that thread calling std::exit. The destruction of all constructed objects with thread storage duration within that thread strongly happens before destroying any object with static storage duration.

we know that the destructor for this_thread will be called before those of any objects of static storage duration. This explains the error message seen in #7 example 1, as after calling the destructor of Vec the snippet above is trying to lock a mutex that has already been destroyed. Since Thead_Profile has a non-trivial destructor, it is undefined behaviour to use it after destruction and I therefore added a boolean flag to track whether or not its destructor has been called, and to avoid using the object if it has. This should prevent crashes but will not allow for complete allocation tracking. A workaround might be to move the Thread_Profile out of TLS and instead store it directly within the map, e.g.

    static inline Map<Thread::Id, Box<Thread_Profile>, Mhidden> threads;

but that also would require some other changes, so in this PR I implemented a simpler solution that only addresses crashing.

The second example reveals another issue with destruction order, this time with the static data contained in impl/log.cpp. In this case when the lock on this_thread.frames_lock fails and an attempt is made to write an error message from die,

rpp/rpp/pos/thread_pos.cpp

Lines 211 to 215 in 29ad6ef

	void Mutex::lock() noexcept {
	int ret = pthread_mutex_lock(&lock_);
	if(ret) {
	die("Failed to lock mutex: %", error(ret));
	}

the implementation of output tries to lock another mutex, g_log_data.lock, which is an object of static storage duration.

rpp/rpp/impl/log.cpp

Line 150 in 29ad6ef

Thread::Lock lock(g_log_data.lock);

Because there is no set order of initialization or destruction of static variables between TUs, it is possible that the destructor of g_log_data will be called before the destructor of the global Vec object in the TU containing main, and that seems to have happened here. The stack trace shows that the attempt to lock g_log_data.lock fails, and die is called again to report the failure, resulting in recursion until an eventual segfault.

I made sure that g_log_data is constructed before its first use and destroyed after its last use by placing an inline variable in log.h which wraps the constructor and destructor of Static_Data. This guarantees that any static object appearing in a TU after the inclusion of log.h will be constructed after g_log_data and destroyed before it.

[TheNumbat]

Thanks for the PR! I think checking this_thread_destroyed is fine; it's not important to log allocation events from static destructors in the thread profile.

I've updated the log initializer to use Storage<> and added a test for static allocating/logging. I also fixed (most of) the problems with log reentrancy on failure.