-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Level object memory leak on world unload #24
Comments
You finally found the reason 👍 I think the first solution is more viable. |
@legoboy0215 I've known for a while what the cause was, but spent a while trying to work out ways to fix it. I couldn't come up with a good way to do it, so I opened this. |
I have noticed this on my server as well |
If you have to choose between the two solutions, use the former one. The impact of having a WeakRef in every position is really expensive. I would like to enquire, however, if there is actually a big problem with the use of |
Anyone fancy testing 87ba326 ? |
Issue description
When a world is unloaded, the Level object is not destroyed due to Position objects holding references to it. This causes myriad bugs, the most famous ones being examples like getName() on null (PocketMine/PocketMine-MP#4205), Level::getProvider returning null (iTXTech/Genisys#1908).
The memory leak is very minor, barely worthy of the name "leak" at all. However, the effects are widespread.
Why this happens
Players with spawn points in worlds that are later unloaded will, for example, hold Position objects pointing to their spawn point in the unloaded world. These Position objects hold strong references to the Level object, which is subsequently not destroyed even after everything related to it is closed, most notably the LevelProvider.
The getName() on null bug occurs when a player with a spawn point in an unloaded world either attempts to teleport back to their spawn point, or when they quit from the server and their data is saved.
Steps to reproduce the issue
Possible solutions
Use Level ID
Instead of holding a direct Level reference, Position objects could instead keep the level ID of the level that it points to. Then, when getLevel() is called on a Position object, obtain a Level reference from the server directly. If the world is an unloaded world, this will return null.
Problems
Use WeakRef
Use WeakRef to hold a Level weak reference instead of a strong one. This would eliminate the problem, but would bring back some performance issues which are the reason Position weak references were removed before now (they previously did use weak references).
Problems
OS and versions
The text was updated successfully, but these errors were encountered: