Incorrect instance reset crash fix. #585
Open
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.
🍰 Pullrequest
See cmangos/mangos-tbc#698
This PR fixes crashes when resetting an instance ID that previously belonged to a dungeon but was re-used as a battleground.
Proof
On server start in https://github.com/cmangos/mangos-tbc/blob/910d594493c2bd715efdba876cb3fd5f490a7cd3/src/game/World/World.cpp#L978 instance ID's are first cleaned up before they are 'packed' which means they are lowered to remove any gaps. The problem however is that besides cleaning up this method also starts a reset countdown timer for instances that are slated to reset but have not done so yet. This timer is started for the original ID's and not modified along with the database content during the packing.
The end result is that an instance id 10 which is slated to expire after server start can be lowered to 5 while a new battleground will re-use the 10 which causes a assert. More detail is given in the fixed issue.
While moving the packing in front of the cleaning results in a slightly less efficient packing as some instances will be removed leaving some holes, it is simpler and less error prone than modifying the reset timers during the packing.
Issues
How2Test
Start a few instances during a server and make some expire (by breaking up the groups) such that gaps are created in the instance ID's after server shutdown.
Now before the remaining instances are slated to reset start the server and notice that the original instance ID's are modified in the database to a lower number. Now start battlegrounds such that the original ID's are re-used. Keep the battleground active until the original dungeon was slated to reset and watch the server crash (or not crash with this fix).