This repository has been archived by the owner on Aug 8, 2023. It is now read-only.
[core] Fix GeometryTile race condition #11570
Closed
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
|
I think we should bite the bullet and do #10457. It's just too difficult to reason about two phase loading to have confidence we've fixed this for good and there aren't more bugs lurking here. |
Follow up fix for issue #11538. This closes a hole in the logic where: - Layout result 1 arrived - Symbols arrives for result 1, it was marked ready for commit - Layout result 2 arrived, clearing "ready for commit" state - Global placement ran on symbols for result 1, but layout result 1 data wasn't committed because it had been replaced with layout result 2.
ChrisLoer
force-pushed
the
cloer-11538-mulligan
branch
from
a134485
to
3489fc0
Compare
|
I'm working on a second PR that tries fixing this via #10457. I'm wary of introducing new problems by tugging on all that logic, but I'll try working through it to see how manageable it feels. |
|
Abandoned in favor of #11575. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
I was almost there with the first fix, but it still left a hole open. @ansis, would you have time to put some eyes on this? The timing is really tricky to think about.
The good news is that @friedbunny set me up with a branch where I could reproduce easily, log what was happening, and verify that this fix made the crash go away.
Follow up fix for issue #11538.
This closes a hole in the logic where:
/cc @jfirebaugh @ansis @friedbunny @julianrex @tobrun