Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapt to setup function additions #2563

Merged
merged 2 commits into from May 31, 2023
Merged

Adapt to setup function additions #2563

merged 2 commits into from May 31, 2023

Conversation

kcgen
Copy link
Member

@kcgen kcgen commented May 31, 2023

The addition of the checked STL flags (on Linux) caused crashes inside the Setup class' ExecuteInit().

The crash happens because the container that ExecuteInit is looping over gets modified (appended to) mid-iteration, which invalidates the loop iterator.

A prior attempt to avoid this moved the appended chunk to a separate container (EarlyInit), however this broke the startup sequence for the mapper.

A prior attempt to solve this also failed:

This PR is now a third attempt to handle the modified container (and avoid crashing with the checked STL) while also keeping the original mapper placement so the startup sequence/functionality is retained.

kcgen added 2 commits May 31, 2023 07:58
We can now safely go back to the original startup
sequence using the normal init function container now
that the execution loop is safely handling additions
(and won't triggering the checked-STL crash in debug
builds).
@kcgen kcgen added the regression We broke something 😊 label May 31, 2023
@kcgen kcgen self-assigned this May 31, 2023
@kcgen
Copy link
Member Author

kcgen commented May 31, 2023

Thanks for the tests, @GranMinigun. Also working here and passing regression tests w/ the mapper. Given this is impacting most builds, let's get this merged ASAP.

@kcgen kcgen merged commit 33c7621 into main May 31, 2023
64 checks passed
@kcgen kcgen deleted the kc/check-setup-additions-1 branch May 31, 2023 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression We broke something 😊
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants