fix: concurrency crash in closeAllShopWindows caused by use-after-free #3211
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
Fixed a concurrency crash in the `closeAllShopWindows` method. The issue occurred when the `shopPlayers` map was cleared while being iterated over, leading to a use-after-free scenario. This fix ensures safe access by retrieving the player object before performing any operations and checking for its validity. This prevents memory access errors and improves the stability of the system in concurrent scenarios.
beats-dh
approved these changes
Jan 4, 2025
majestyotbr
approved these changes
Jan 4, 2025
majestyotbr
deleted the
dudantas/fix-crash-related-to-npc-close-shop-window
branch
|
vllworldbuilding
pushed a commit
to vllworldbuilding/canary
that referenced
this pull request
Jan 10, 2025
opentibiabr#3211) This fixes a concurrency crash in the `closeAllShopWindows` method. The issue occurred when the `shopPlayers` map was cleared while being iterated over, leading to a use-after-free scenario. The solution removes the use of references when iterating over `shopPlayers` keys, ensuring that the keys are copied instead of directly referencing the map. This change prevents crashes and improves stability in concurrent environments.
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.
Description
This PR fixes a concurrency crash in the
closeAllShopWindowsmethod. The issue occurred when theshopPlayersmap was cleared while being iterated over, leading to a use-after-free scenario. The solution removes the use of references when iterating overshopPlayerskeys, ensuring that the keys are copied instead of directly referencing the map. This change prevents crashes and improves stability in concurrent environments.Behaviour
Actual
shopPlayerswith references caused a crash in some scenarios because the map was cleared during the iteration, leading to use-after-free memory access.Expected
shopPlayersno longer results in crashes because the keys are copied, ensuring safe iteration even if the map is modified during the process.Type of change
How Has This Been Tested
shopPlayersis cleared while being iterated to validate the fix.closeAllShopWindowsunder concurrent conditions without crashes.Checklist