Fixes crash in ClassDB::deinitialize due to usage of invalid iterator.

[Klaim]

I took a stab at trying the hot-reloading system (in master) but when using my experimental/prototyping GDExtension (which exposes 2 Node types) but hit a bug:
When opening Godot (4.2-dev6) which loaded my extension (built with cd61a9b) my extension appears to run perfectly well. Closing Godot at any time will trigger an error due to invalid access to std::vector internals. Exact same issue when attempting a hot-reloading, at the moment we get the focus after changing the extension binary.
Note that this does not happen in the example provided in this project, but removing any active code or even usage in the project of my GDExtension , except the node classes registrations (2 types) systematically reproduces the issue for now (it's UB so it's random, see below).

As my extension and godot-cpp were built in debug, I quickly spotted that the issue was in the ClassDB::deinitialize function, after the first iteration.

Reading the code I spotted that line:

godot-cpp/src/core/class_db.cpp

Line 370 in cd61a9b

class_register_order.erase((i + 1).base());

This call to std::vector::erase invalidates any iterator (including i) and the result can be random because it's Undefiend Behavior. We dont know if the vector will or not re-allocate it's internal array. So the whole loop is incorrect as long as the iterator is used after that erase.

There is usually two ways to solve this issue:
A) do like I did in this PR, that is, collect a way to identify the elements to remove, then remove them after the pass detecting what to remove;
B) std::vector::erase returns an iterator to the next valid iterator, use that.

Solution A):

This change works for me/my project when I use it in my copy of godot-cpp , but it needs eyes to be sure (because UB will not be easilly checked).
Also more expensive than B because we have to have a second container to grow. I chose a set for convenience but it's definitely not the best performance wise for this kind of scenario - if you have some custom container that would fit better feel free to suggest them to me. I'm thinking something like a flat_set or stack-first-set/vector or something like that.

Also note that I dont know why it was necessary to go in reverse, nor do I know why i + 1 was used here. Feel free to inform me on this, I failed to find how/where to chat with the authors.

Solution B):

I attempted to use the iterator from erase but it's a normal iterator and originally we were going with reverse iterators. I need to check how to obtain a reverse iterator from an iterator. I suspect this solution to be less expensive in both time and speed, just didnt manage to make it work yet.

Resolved

[dsnopek]

Thanks!

[dsnopek]

The code is looking good to me now! However, this'll need to be squashed into a single commit before it can be merged, and we need to wait to see what the CI says.

[Klaim]

No problem, I was not sure if you want to do it automatically or I should do it. Incoming.

[dsnopek]

Thanks! And congrats on your first merged PR! 🎉 🚀