Skip to content

Conversation

@Sonicadvance1
Copy link
Member

Even though we leak Alloc64 on shutdown, that isn't good enough to avoid the atexit handler deallocating memory. So we need to use a raw pointer and leak it.

Removes an atexit registration that contributes to crashing on exit.

Even though we leak `Alloc64` on shutdown, that isn't good enough to
avoid the `atexit` handler deallocating memory. So we need to use a raw
pointer and leak it.

Removes an `atexit` registration that contributes to crashing on exit.
@neobrain
Copy link
Member

Even though we leak Alloc64 on shutdown, that isn't good enough to avoid the atexit handler deallocating memory.

What exactly is happening there? Is this something specific about unique_ptr? Or does glibc somehow iterate over all pending allocations in atexit (why would it need to though?) ?

@Sonicadvance1
Copy link
Member Author

Even though we leak Alloc64 on shutdown, that isn't good enough to avoid the atexit handler deallocating memory.

What exactly is happening there? Is this something specific about unique_ptr? Or does glibc somehow iterate over all pending allocations in atexit (why would it need to though?) ?

Any static lifetime constructor member that allocates memory registers an atexit handler in order to destruct memory.
Static lifetime constructors will allocate memory before FEX has installed its hooks, once a host-side atexit handler is called, the handlers will try to deallocate memory and explode because now an incorrect allocator is trying to deallocate that memory.

We must remove all of FEX's global static constructors that allocate memory to mitigate the problem.

@neobrain
Copy link
Member

Any static lifetime constructor member that allocates memory registers an atexit handler in order to destruct memory.

So global variables that have destructors are fine in principle, but if the constructor allocates memory it's not? But where would std::unique_ptr's default constructor allocate memory? And why would its destructor try to de-allocate when we release() the associated pointer already?

@Sonicadvance1
Copy link
Member Author

Any static lifetime constructor member that allocates memory registers an atexit handler in order to destruct memory.

So global variables that have destructors are fine in principle, but if the constructor allocates memory it's not? But where would std::unique_ptr's default constructor allocate memory? And why would its destructor try to de-allocate when we release() the associated pointer already?

In this PR's example:
fextl::unique_ptr<Alloc::HostAllocator> Alloc64 {};

This doesn't allocate memory, but it does register an atexit handler. This exit handler then deallocates memory that had been allocated from SetupHooks. This has the same failure mode why we have the workaround for with Alloc::OSAllocator::ReleaseAllocatorWorkaround(std::move(Alloc64));

In something like #5214,

static std::array<fextl::string, Paths::PATH_LAST> Paths;
static fextl::map<FEXCore::Config::LayerType, fextl::unique_ptr<FEXCore::Config::Layer>> ConfigLayers;

These also both register atexit handlers to free their contained data. Which will contain data from multiple VMA allocators. Once the exit handlers are called then things crash.

I've already removed all the static objects that up-front allocate over the last few months because it was causing spurious glibc fault CI failures depending on allocation order. But to ensure we don't have shutdown failure we need to ensure we don't have any atexit handlers that deallocate container memory. Once all of these are removed I'll be adding a CI validation step to ensure nothing registers an atexit handler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants