PocketMine / PocketMine-MP

Legacy PocketMine-MP repository. Head to https://github.com/pmmp for up to date software.
https://www.pocketmine.net/
GNU Lesser General Public License v3.0
1.26k stars 661 forks source link

Fix a getName() on null bug #4205

Closed robske110 closed 7 years ago

robske110 commented 8 years ago

I don't know exactly when this is happening. But it seems to happen if a player's spawnposition is in a level wich was previously loaded, but then unloaded. (Then the next autosave gets called => save() gets a level wich is closed therefore the provider has became (probably even destructed) null wich triggers this getName() on null.) We should consider checking on all level get functions that are using the provider variable to check if the provider is not null (level not closed) for less plugin crashes

Here is a Traceback wich helped me:

2016-05-29 [19:07:55] [Server thread/CRITICAL]: Error: "Call to a member function getName() on null" (EXCEPTION) in "/src/pocketmine/level/Level" at line 2995
2016-05-29 [19:07:55] [Server thread/DEBUG]: #0 /src/pocketmine/Player(3989): pocketmine\Player->save(boolean)
2016-05-29 [19:07:55] [Server thread/DEBUG]: #1 /src/pocketmine/Server(2473): pocketmine\Player->close(pocketmine\event\TranslationContainer ..e%multiplayer.player.left, string Server closed)
2016-05-29 [19:07:55] [Server thread/DEBUG]: #2 /src/pocketmine/Server(2670): pocketmine\Server->forceShutdown(boolean)
2016-05-29 [19:07:55] [Server thread/DEBUG]: #3 /src/pocketmine/Server(2611): pocketmine\Server->crashDump(boolean)
2016-05-29 [19:07:55] [Server thread/DEBUG]: #4 /src/pocketmine/Server(2078): pocketmine\Server->exceptionHandler(Error Error: Call to a member function getName() on null in phar:///home/pm1/PocketMine-MP.phar/src/pocketmine/level/Level.php:2995.Stack trace:.#0 phar:///home/pm1/PocketMine-MP.phar/src/pocketmine/Player.php(4076): pocketmine\level\Level->getName().#1 phar:///home/pm1/PocketMine-MP.phar/src/pocketmine/Server.php(2829): pocketmine\Player->save(true).#2 phar:///home/pm1/PocketMine-MP.phar/src/pocketmine/Server.php(3015): pocketmine\Server->doAutoSave().#3 phar:///home/pm1/PocketMine-MP.phar/src/pocketmine/Server.php(2683): pocketmine\Server->tick().#4 phar:///home/pm1/PocketMine-MP.phar/src/pocketmine/Server.php(2561): pocketmine\Server->tickProcessor().#5 phar:///home/pm1/PocketMine-MP.phar/src/pocketmine/Server.php(2076): pocketmine\Server->start().#6 phar:///home/pm1/PocketMine-MP.phar/src/pocketmine/PocketMine.php(467): pocketmine\Server->__construct(Object(pocketmine\CompatibleClassLoader), Object(pocketmine\utils\MainLogger), 'phar:///home/pm...', '/home/pm1/', '/home/pm1/plugi...', 'unknown').#7 /home/pm1/PocketMine-MP.phar(1): require_once('phar:///home/pm...').#8 {main})
2016-05-29 [19:07:55] [Server thread/DEBUG]: #5 /src/pocketmine/PocketMine(467): pocketmine\Server->__construct(pocketmine\CompatibleClassLoader object, pocketmine\utils\MainLogger object, string phar:///home/pm1/PocketMine-MP.phar/, string /home/pm1/, string /home/pm1/plugins/, string unknown)
2016-05-29 [19:07:55] [Server thread/DEBUG]: #6 (1): require_once(string phar:///home/pm1/PocketMine-MP.phar/src/pocketmine/PocketMine.php)
2016-05-29 [19:07:55] [Server thread/EMERGENCY]: Crashed while crashing, killing process
ghost commented 8 years ago

xD 2016-05-29 [19:07:55] [Server thread/EMERGENCY]: Crashed while crashing, killing process How can it crash while crashing? Anyways, I tried this out and it works good. Thanks! It should be merged

SOF3 commented 8 years ago

@XtremePlayzCODE while handling a critical error (more precisely, after handling a critical error and trying to gracefully stop the server), another critical error happened.

Therefore, this issue may be caused by the critical error that caused the initial crash, which may have led to the LevelProvider being null.

Also note that PocketMine code style prefers the use of !== null to !== NULL.

robske110 commented 8 years ago

@SOF3 Yes, the example crash happened while forcefully stopping the server. But this error can happen while normal running too, i explained th conditions in my inital post.

To that !== null, I will change that, i hope to not mess this thing up with rebase

SOF3 commented 8 years ago

It feels strange if you add the !== null for every single call. Just like the hasPermission() crash about kicking players in some events. Those calls simply shouldn't have happened at all, so returning dummy values isn't a proper way to fix it. Meanwhile checking if player is kicked after every of these event calls seems to be very strange.

This is really a debatable problem in software engineering.

robske110 commented 8 years ago

Maybe, well i have no real idea Maybe just make all level functions wich use the level provider checking if it is null?

SOF3 commented 8 years ago

@robske110 that's what I was saying: will be very strange to do some pretty unnecessary/minor checks in every single usage. Affects code quality and appearance overall.

robske110 commented 7 years ago

Closing due to fixed in #pmmp :)