Fix crash when saving resources with circular references #68281
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Any idea why the build tests are failing? |
maximkulkin
force-pushed
the
resource-circular-references
branch
from
80cc78d
to
4ec5bb4
Compare
|
I have pulled back support for saving circular references as it can cause memory leaks. Instead I narrowed this change down to fixing crash when saving resources with circular references. |
maximkulkin
changed the title
akien-mga
reviewed
Feb 17, 2023
maximkulkin
force-pushed
the
resource-circular-references
branch
from
4ec5bb4
to
5c8fde4
Compare
|
Looks good to me overall, but needs a review from @reduz to make sure it's correct. |
maximkulkin
force-pushed
the
resource-circular-references
branch
3 times, most recently
from
381035d
to
f39b9cd
Compare
RandomShaper
approved these changes
Jun 20, 2023
When saving resources, marking of already seen resources was done too late, causing infinite loop traversing referenced resources and eventual stack overflow. The change marks traversed resource before descending to it's children, thus when this resource is encountered again, it is already marked as seen and traversal stops.
YuriSizov
force-pushed
the
resource-circular-references
branch
from
f39b9cd
to
058604f
Compare
|
I rebased and force-pushed it on your behalf because it's been many months since the last update. Just in case there may be problems or new checks on CI. |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When saving resources, marking of already seen resources was done too late, causing infinite loop traversing referenced resources and eventual stack overflow. The change marks traversed resource before descending to it's children, thus when this resource is encountered again, it is already marked as seen and traversal stops.
Fixes #60590