[GEN][ZH] Fix Memory Manager initialization issues (old)

[xezon]

• Fixes Fix debug memory crash on exit in VS22 due to recent TheGlobalData refactor #1203
• Follow up for [GEN][ZH] Refactor and clean up Command Line parsing #1068

This change fixes Memory Manager initialization issues.

Problems identified: ClientInstance::initialize and CommandLine::parseCommandLineForStartup both used 'new' allocator before the Memory Manager was initialized, in both Debug and Release. This caused the Memory Manager to actually be created twice, and trigger 1 assert on game launch and 1 assert on game quit.

This is now fixed by fixing initialization ordering and preventing some allocations with 'new'.

This change also uses the stl::malloc_allocator that was added to the pending Pull Request #1066.

TODO

• Replicate in Generals

[helmutbuhler]

Thanks for investigating this! Your change does indeed fix the issue, but it also adds a lot of complexity. Adding yet another phase to commandline parsing is not the right way to go in my opinion (I think even two is one too many).

I think we can fix this by simply moving the DEBUG_LOG call in preMainInitMemoryManager until after TheMemoryPoolFactory is assigned, thereby avoiding the unwanted recursion.
I tried this out, see the two commits here (based on this branch):
https://github.com/helmutbuhler/CnC_Generals_Zero_Hour/commits/fix-memory-initialization-issues/
It also avoids the need for MallocAllocator.

[xezon]

This was also one of my tries to fix this, but it was not the right approach, because there is also at least one DEBUG_ASSERTCRASH (which implies logging when hit) in the memory manager initialization. Plus client instance initialization also needs to be done before debug logging and therefore without new/delete, because the instance id affects the name of the log file.

This is quite a bit complicated order of initialization but this change fixes it.

Order:

Command Line parse (parts)
Client Instance
Debug Log
Memory Manager
The Rest

MallocAllocator is needed regardless. There will always be things that want to bypass Memory Manager in rare special cases. Memory Manager itself needs to bypass new/delete :-)

[helmutbuhler]

This was also one of my tries to fix this, but it was not the right approach, because there is also at least one DEBUG_ASSERTCRASH (which implies logging when hit) in the memory manager initialization.

Where is this DEBUG_ASSERTCRASH?

[xezon]

DynamicMemoryAllocator::init

[xezon]

We can move Memory Manager first above all but then need to never use logging in it. We could use assert then.

[xezon]

I looked deeper, there are more asserts in

MemoryPoolFactory::createMemoryPool
MemoryPoolFactory::findMemoryPool
userMemoryAdjustPoolSize
MemoryPool::createBlob

Moving logging behind memory manager will lose logging/asserting for all of this.

[xezon]

So yeah we need to decide, is allocation (using new) more important everywhere, or the ability to log/assert everywhere?

[xezon]

What we can try to do is make DEBUG_CRASH more usable when Debug was not yet initialized. Logging nothing. Do you want that?

[xezon]

I am working on a new change. Will be done later.

[helmutbuhler]

DynamicMemoryAllocator::init

Ah, you are right, I must have missed that. Hm.

What we can try to do is make DEBUG_CRASH more usable when Debug was not yet initialized. Logging nothing. Do you want that?

Yes, that sounds pretty good.

Another alternative might be to remove/replace all globals that allocate, not sure if that's feasable.

[helmutbuhler]

Another alternative might be to remove/replace all globals that allocate, not sure if that's feasable.

Never mind, there are so many globals...

[xezon]

Done: #1275

[xezon]

Obsoleted