diff --git a/src/Server.php b/src/Server.php index e8f2d2726..148c93b8e 100644 --- a/src/Server.php +++ b/src/Server.php @@ -93,6 +93,7 @@ use pocketmine\scheduler\AsyncPool; use pocketmine\snooze\SleeperHandler; use pocketmine\stats\SendUsageTask; use pocketmine\thread\log\AttachableThreadSafeLogger; +use pocketmine\thread\ThreadCrashException; use pocketmine\thread\ThreadSafeClassLoader; use pocketmine\timings\Timings; use pocketmine\timings\TimingsHandler; @@ -1516,23 +1517,38 @@ class Server{ $trace = $e->getTrace(); } - $errstr = $e->getMessage(); - $errfile = $e->getFile(); - $errline = $e->getLine(); + //If this is a thread crash, this logs where the exception came from on the main thread, as opposed to the + //crashed thread. This is intentional, and might be useful for debugging + //Assume that the thread already logged the original exception with the correct stack trace + $this->logger->logException($e, $trace); + + if($e instanceof ThreadCrashException){ + $info = $e->getCrashInfo(); + $type = $info->getType(); + $errstr = $info->getMessage(); + $errfile = $info->getFile(); + $errline = $info->getLine(); + $printableTrace = $info->getTrace(); + $thread = $info->getThreadName(); + }else{ + $type = get_class($e); + $errstr = $e->getMessage(); + $errfile = $e->getFile(); + $errline = $e->getLine(); + $printableTrace = Utils::printableTraceWithMetadata($trace); + $thread = "Main"; + } $errstr = preg_replace('/\s+/', ' ', trim($errstr)); - $errfile = Filesystem::cleanPath($errfile); - - $this->logger->logException($e, $trace); - $lastError = [ - "type" => get_class($e), + "type" => $type, "message" => $errstr, - "fullFile" => $e->getFile(), - "file" => $errfile, + "fullFile" => $errfile, + "file" => Filesystem::cleanPath($errfile), "line" => $errline, - "trace" => $trace + "trace" => $printableTrace, + "thread" => $thread ]; global $lastExceptionError, $lastError; diff --git a/src/crash/CrashDump.php b/src/crash/CrashDump.php index d7223eb2f..40af53fd0 100644 --- a/src/crash/CrashDump.php +++ b/src/crash/CrashDump.php @@ -29,11 +29,13 @@ use pocketmine\network\mcpe\protocol\ProtocolInfo; use pocketmine\plugin\PluginBase; use pocketmine\plugin\PluginManager; use pocketmine\Server; +use pocketmine\thread\ThreadCrashInfoFrame; use pocketmine\utils\AssumptionFailedError; use pocketmine\utils\Filesystem; use pocketmine\utils\Utils; use pocketmine\VersionInfo; use Symfony\Component\Filesystem\Path; +use function array_map; use function base64_encode; use function error_get_last; use function file; @@ -186,7 +188,7 @@ class CrashDump{ if($error === null){ throw new \RuntimeException("Crash error information missing - did something use exit()?"); } - $error["trace"] = Utils::currentTrace(3); //Skipping CrashDump->baseCrash, CrashDump->construct, Server->crashDump + $error["trace"] = Utils::printableTrace(Utils::currentTrace(3)); //Skipping CrashDump->baseCrash, CrashDump->construct, Server->crashDump $error["fullFile"] = $error["file"]; $error["file"] = Filesystem::cleanPath($error["file"]); try{ @@ -201,9 +203,6 @@ class CrashDump{ $error["message"] = mb_scrub($error["message"], 'UTF-8'); if(isset($lastError)){ - if(isset($lastError["trace"])){ - $lastError["trace"] = Utils::printableTrace($lastError["trace"]); - } $this->data->lastError = $lastError; $this->data->lastError["message"] = mb_scrub($this->data->lastError["message"], 'UTF-8'); } @@ -215,10 +214,11 @@ class CrashDump{ $this->data->plugin_involvement = self::PLUGIN_INVOLVEMENT_NONE; if(!$this->determinePluginFromFile($error["fullFile"], true)){ //fatal errors won't leave any stack trace foreach($error["trace"] as $frame){ - if(!isset($frame["file"])){ + $frameFile = $frame->getFile(); + if($frameFile === null){ continue; //PHP core } - if($this->determinePluginFromFile($frame["file"], false)){ + if($this->determinePluginFromFile($frameFile, false)){ break; } } @@ -233,7 +233,8 @@ class CrashDump{ } } - $this->data->trace = Utils::printableTrace($error["trace"]); + $this->data->trace = array_map(array: $error["trace"], callback: fn(ThreadCrashInfoFrame $frame) => $frame->getPrintableFrame()); + $this->data->thread = $error["thread"]; } private function determinePluginFromFile(string $filePath, bool $crashFrame) : bool{ diff --git a/src/crash/CrashDumpData.php b/src/crash/CrashDumpData.php index 0f5358be5..b71e6f405 100644 --- a/src/crash/CrashDumpData.php +++ b/src/crash/CrashDumpData.php @@ -37,6 +37,8 @@ final class CrashDumpData implements \JsonSerializable{ /** @var mixed[] */ public array $error; + public string $thread; + public string $plugin_involvement; public string $plugin = ""; diff --git a/src/crash/CrashDumpRenderer.php b/src/crash/CrashDumpRenderer.php index 2858f43ec..617dcb7ab 100644 --- a/src/crash/CrashDumpRenderer.php +++ b/src/crash/CrashDumpRenderer.php @@ -64,6 +64,7 @@ final class CrashDumpRenderer{ $this->addLine(); + $this->addLine("Thread: " . $this->data->thread); $this->addLine("Error: " . $this->data->error["message"]); $this->addLine("File: " . $this->data->error["file"]); $this->addLine("Line: " . $this->data->error["line"]); diff --git a/src/network/mcpe/raklib/RakLibInterface.php b/src/network/mcpe/raklib/RakLibInterface.php index 93dff68a2..4bf8ffb15 100644 --- a/src/network/mcpe/raklib/RakLibInterface.php +++ b/src/network/mcpe/raklib/RakLibInterface.php @@ -39,6 +39,7 @@ use pocketmine\network\NetworkInterfaceStartException; use pocketmine\network\PacketHandlingException; use pocketmine\player\GameMode; use pocketmine\Server; +use pocketmine\thread\ThreadCrashException; use pocketmine\timings\Timings; use pocketmine\utils\Utils; use raklib\generic\DisconnectReason; @@ -154,7 +155,7 @@ class RakLibInterface implements ServerEventListener, AdvancedNetworkInterface{ if(!$this->rakLib->isRunning()){ $e = $this->rakLib->getCrashInfo(); if($e !== null){ - throw new \RuntimeException("RakLib crashed: " . $e->makePrettyMessage()); + throw new ThreadCrashException("RakLib crashed", $e); } throw new \Exception("RakLib Thread crashed without crash information"); } diff --git a/src/network/mcpe/raklib/RakLibServer.php b/src/network/mcpe/raklib/RakLibServer.php index a3f7c1609..e59b6971f 100644 --- a/src/network/mcpe/raklib/RakLibServer.php +++ b/src/network/mcpe/raklib/RakLibServer.php @@ -29,6 +29,7 @@ use pocketmine\snooze\SleeperHandlerEntry; use pocketmine\thread\log\ThreadSafeLogger; use pocketmine\thread\NonThreadSafeValue; use pocketmine\thread\Thread; +use pocketmine\thread\ThreadCrashException; use raklib\generic\SocketException; use raklib\server\ipc\RakLibToUserThreadMessageSender; use raklib\server\ipc\UserToRakLibThreadMessageReceiver; @@ -37,17 +38,12 @@ use raklib\server\ServerSocket; use raklib\server\SimpleProtocolAcceptor; use raklib\utils\ExceptionTraceCleaner; use raklib\utils\InternetAddress; -use function error_get_last; use function gc_enable; use function ini_set; -use function register_shutdown_function; class RakLibServer extends Thread{ - protected bool $cleanShutdown = false; protected bool $ready = false; protected string $mainPath; - /** @phpstan-var NonThreadSafeValue|null */ - public ?NonThreadSafeValue $crashInfo = null; /** @phpstan-var NonThreadSafeValue */ protected NonThreadSafeValue $address; @@ -69,86 +65,51 @@ class RakLibServer extends Thread{ $this->address = new NonThreadSafeValue($address); } - /** - * @return void - */ - public function shutdownHandler(){ - if($this->cleanShutdown !== true && $this->crashInfo === null){ - $error = error_get_last(); - - if($error !== null){ - $this->logger->emergency("Fatal error: " . $error["message"] . " in " . $error["file"] . " on line " . $error["line"]); - $this->setCrashInfo(RakLibThreadCrashInfo::fromLastErrorInfo($error)); - }else{ - $this->logger->emergency("RakLib shutdown unexpectedly"); - } - } - } - - public function getCrashInfo() : ?RakLibThreadCrashInfo{ - return $this->crashInfo?->deserialize(); - } - - private function setCrashInfo(RakLibThreadCrashInfo $info) : void{ - $this->synchronized(function() use ($info) : void{ - $this->crashInfo = new NonThreadSafeValue($info); - $this->notify(); - }); - } - public function startAndWait(int $options = NativeThread::INHERIT_NONE) : void{ $this->start($options); $this->synchronized(function() : void{ - while(!$this->ready && $this->crashInfo === null){ + while(!$this->ready && $this->getCrashInfo() === null){ $this->wait(); } - $crashInfo = $this->crashInfo?->deserialize(); + $crashInfo = $this->getCrashInfo(); if($crashInfo !== null){ - if($crashInfo->getClass() === SocketException::class){ + if($crashInfo->getType() === SocketException::class){ throw new SocketException($crashInfo->getMessage()); } - throw new \RuntimeException("RakLib failed to start: " . $crashInfo->makePrettyMessage()); + throw new ThreadCrashException("RakLib failed to start", $crashInfo); } }); } protected function onRun() : void{ - try{ - gc_enable(); - ini_set("display_errors", '1'); - ini_set("display_startup_errors", '1'); + gc_enable(); + ini_set("display_errors", '1'); + ini_set("display_startup_errors", '1'); - register_shutdown_function([$this, "shutdownHandler"]); - - try{ - $socket = new ServerSocket($this->address->deserialize()); - }catch(SocketException $e){ - $this->setCrashInfo(RakLibThreadCrashInfo::fromThrowable($e)); - return; - } - $manager = new Server( - $this->serverId, - $this->logger, - $socket, - $this->maxMtuSize, - new SimpleProtocolAcceptor($this->protocolVersion), - new UserToRakLibThreadMessageReceiver(new PthreadsChannelReader($this->mainToThreadBuffer)), - new RakLibToUserThreadMessageSender(new SnoozeAwarePthreadsChannelWriter($this->threadToMainBuffer, $this->sleeperEntry->createNotifier())), - new ExceptionTraceCleaner($this->mainPath) - ); - $this->synchronized(function() : void{ - $this->ready = true; - $this->notify(); - }); - while(!$this->isKilled){ - $manager->tickProcessor(); - } - $manager->waitShutdown(); - $this->cleanShutdown = true; - }catch(\Throwable $e){ - $this->setCrashInfo(RakLibThreadCrashInfo::fromThrowable($e)); - $this->logger->logException($e); + $socket = new ServerSocket($this->address->deserialize()); + $manager = new Server( + $this->serverId, + $this->logger, + $socket, + $this->maxMtuSize, + new SimpleProtocolAcceptor($this->protocolVersion), + new UserToRakLibThreadMessageReceiver(new PthreadsChannelReader($this->mainToThreadBuffer)), + new RakLibToUserThreadMessageSender(new SnoozeAwarePthreadsChannelWriter($this->threadToMainBuffer, $this->sleeperEntry->createNotifier())), + new ExceptionTraceCleaner($this->mainPath) + ); + $this->synchronized(function() : void{ + $this->ready = true; + $this->notify(); + }); + while(!$this->isKilled){ + $manager->tickProcessor(); } + $manager->waitShutdown(); + } + + protected function onUncaughtException(\Throwable $e) : void{ + parent::onUncaughtException($e); + $this->logger->logException($e); } public function getThreadName() : string{ diff --git a/src/network/mcpe/raklib/RakLibThreadCrashInfo.php b/src/network/mcpe/raklib/RakLibThreadCrashInfo.php deleted file mode 100644 index 60e04b4b4..000000000 --- a/src/network/mcpe/raklib/RakLibThreadCrashInfo.php +++ /dev/null @@ -1,61 +0,0 @@ -getMessage(), $e->getFile(), $e->getLine()); - } - - /** - * @phpstan-param array{message: string, file: string, line: int} $info - */ - public static function fromLastErrorInfo(array $info) : self{ - return new self(null, $info["message"], $info["file"], $info["line"]); - } - - public function getClass() : ?string{ return $this->class; } - - public function getMessage() : string{ return $this->message; } - - public function getFile() : string{ return $this->file; } - - public function getLine() : int{ return $this->line; } - - public function makePrettyMessage() : string{ - return sprintf("%s: \"%s\" in %s on line %d", $this->class ?? "Fatal error", $this->message, Filesystem::cleanPath($this->file), $this->line); - } -} diff --git a/src/scheduler/AsyncPool.php b/src/scheduler/AsyncPool.php index 7540294ef..bb79df507 100644 --- a/src/scheduler/AsyncPool.php +++ b/src/scheduler/AsyncPool.php @@ -26,6 +26,7 @@ namespace pocketmine\scheduler; use pmmp\thread\Thread as NativeThread; use pocketmine\snooze\SleeperHandler; use pocketmine\thread\log\ThreadSafeLogger; +use pocketmine\thread\ThreadCrashException; use pocketmine\thread\ThreadSafeClassLoader; use pocketmine\timings\Timings; use pocketmine\utils\AssumptionFailedError; @@ -215,12 +216,17 @@ class AsyncPool{ } } } - if($crashedTask !== null){ - $message = "Worker $workerId crashed while running task " . get_class($crashedTask) . "#" . spl_object_id($crashedTask); + $info = $entry->worker->getCrashInfo(); + if($info !== null){ + if($crashedTask !== null){ + $message = "Worker $workerId crashed while running task " . get_class($crashedTask) . "#" . spl_object_id($crashedTask); + }else{ + $message = "Worker $workerId crashed while doing unknown work"; + } + throw new ThreadCrashException($message, $info); }else{ - $message = "Worker $workerId crashed for unknown reason"; + throw new \RuntimeException("Worker $workerId crashed for unknown reason"); } - throw new \RuntimeException($message); } } diff --git a/src/scheduler/AsyncWorker.php b/src/scheduler/AsyncWorker.php index 19a19b102..b26afc29b 100644 --- a/src/scheduler/AsyncWorker.php +++ b/src/scheduler/AsyncWorker.php @@ -31,7 +31,6 @@ use pocketmine\thread\Worker; use pocketmine\utils\AssumptionFailedError; use function gc_enable; use function ini_set; -use function set_exception_handler; class AsyncWorker extends Worker{ /** @var mixed[] */ @@ -68,20 +67,17 @@ class AsyncWorker extends Worker{ } $this->saveToThreadStore(self::TLS_KEY_NOTIFIER, $this->sleeperEntry->createNotifier()); + } - set_exception_handler(function(\Throwable $e){ - $this->logger->logException($e); - }); + protected function onUncaughtException(\Throwable $e) : void{ + parent::onUncaughtException($e); + $this->logger->logException($e); } public function getLogger() : ThreadSafeLogger{ return $this->logger; } - public function handleException(\Throwable $e) : void{ - $this->logger->logException($e); - } - public function getThreadName() : string{ return "AsyncWorker#" . $this->id; } diff --git a/src/thread/CommonThreadPartsTrait.php b/src/thread/CommonThreadPartsTrait.php index c35dd7791..340ce554d 100644 --- a/src/thread/CommonThreadPartsTrait.php +++ b/src/thread/CommonThreadPartsTrait.php @@ -26,7 +26,10 @@ namespace pocketmine\thread; use pmmp\thread\ThreadSafeArray; use pocketmine\errorhandler\ErrorToExceptionHandler; use pocketmine\Server; +use function error_get_last; use function error_reporting; +use function register_shutdown_function; +use function set_exception_handler; trait CommonThreadPartsTrait{ /** @@ -38,6 +41,8 @@ trait CommonThreadPartsTrait{ protected bool $isKilled = false; + private ?ThreadCrashInfo $crashInfo = null; + /** * @return ThreadSafeClassLoader[] */ @@ -88,12 +93,48 @@ trait CommonThreadPartsTrait{ } } + public function getCrashInfo() : ?ThreadCrashInfo{ return $this->crashInfo; } + final public function run() : void{ error_reporting(-1); $this->registerClassLoaders(); //set this after the autoloader is registered ErrorToExceptionHandler::set(); + + //this permits adding extra functionality to the exception and shutdown handlers via overriding + set_exception_handler($this->onUncaughtException(...)); + register_shutdown_function($this->onShutdown(...)); + $this->onRun(); + $this->isKilled = true; + } + + /** + * Called by set_exception_handler() when an uncaught exception is thrown. + */ + protected function onUncaughtException(\Throwable $e) : void{ + $this->synchronized(function() use ($e) : void{ + $this->crashInfo = ThreadCrashInfo::fromThrowable($e, $this->getThreadName()); + }); + } + + /** + * Called by register_shutdown_function() when the thread shuts down. This may be because of a benign shutdown, or + * because of a fatal error. Use isKilled to determine which. + */ + protected function onShutdown() : void{ + $this->synchronized(function() : void{ + if(!$this->isKilled && $this->crashInfo === null){ + $last = error_get_last(); + if($last !== null){ + //fatal error + $this->crashInfo = ThreadCrashInfo::fromLastErrorInfo($last, $this->getThreadName()); + }else{ + //probably misused exit() + $this->crashInfo = ThreadCrashInfo::fromThrowable(new \RuntimeException("Thread crashed without an error - perhaps exit() was called?"), $this->getThreadName()); + } + } + }); } /** diff --git a/src/thread/ThreadCrashException.php b/src/thread/ThreadCrashException.php new file mode 100644 index 000000000..0eba37934 --- /dev/null +++ b/src/thread/ThreadCrashException.php @@ -0,0 +1,38 @@ +crashInfo = $crashInfo; + } + + public function getCrashInfo() : ThreadCrashInfo{ + return $this->crashInfo; + } +} diff --git a/src/thread/ThreadCrashInfo.php b/src/thread/ThreadCrashInfo.php new file mode 100644 index 000000000..66aae927a --- /dev/null +++ b/src/thread/ThreadCrashInfo.php @@ -0,0 +1,89 @@ + */ + private ThreadSafeArray $trace; + + /** + * @param ThreadCrashInfoFrame[] $trace + */ + public function __construct( + private string $type, + private string $message, + private string $file, + private int $line, + array $trace, + private string $threadName + ){ + $this->trace = ThreadSafeArray::fromArray($trace); + } + + public static function fromThrowable(\Throwable $e, string $threadName) : self{ + return new self(get_class($e), $e->getMessage(), $e->getFile(), $e->getLine(), Utils::printableTraceWithMetadata($e->getTrace()), $threadName); + } + + /** + * @phpstan-param array{type: int, message: string, file: string, line: int} $info + */ + public static function fromLastErrorInfo(array $info, string $threadName) : self{ + try{ + $class = ErrorTypeToStringMap::get($info["type"]); + }catch(\InvalidArgumentException){ + $class = "Unknown error type (" . $info["type"] . ")"; + } + return new self($class, $info["message"], $info["file"], $info["line"], Utils::printableTraceWithMetadata(Utils::currentTrace()), $threadName); + } + + public function getType() : string{ return $this->type; } + + public function getMessage() : string{ return $this->message; } + + public function getFile() : string{ return $this->file; } + + public function getLine() : int{ return $this->line; } + + /** + * @return ThreadCrashInfoFrame[] + */ + public function getTrace() : array{ + return (array) $this->trace; + } + + public function getThreadName() : string{ return $this->threadName; } + + public function makePrettyMessage() : string{ + return sprintf("%s: \"%s\" in \"%s\" on line %d", $this->type ?? "Fatal error", $this->message, Filesystem::cleanPath($this->file), $this->line); + } +} diff --git a/src/thread/ThreadCrashInfoFrame.php b/src/thread/ThreadCrashInfoFrame.php new file mode 100644 index 000000000..27f470387 --- /dev/null +++ b/src/thread/ThreadCrashInfoFrame.php @@ -0,0 +1,41 @@ +printableFrame; } + + public function getFile() : ?string{ return $this->file; } + + public function getLine() : int{ return $this->line; } +} diff --git a/src/utils/Utils.php b/src/utils/Utils.php index 11548e193..f5ec5f8e4 100644 --- a/src/utils/Utils.php +++ b/src/utils/Utils.php @@ -31,6 +31,7 @@ use DaveRandom\CallbackValidator\CallbackType; use pocketmine\entity\Location; use pocketmine\errorhandler\ErrorTypeToStringMap; use pocketmine\math\Vector3; +use pocketmine\thread\ThreadCrashInfoFrame; use Ramsey\Uuid\Uuid; use Ramsey\Uuid\UuidInterface; use function array_combine; @@ -469,6 +470,30 @@ final class Utils{ return $messages; } + /** + * Similar to {@link Utils::printableTrace()}, but associates metadata such as file and line number with each frame. + * This is used to transmit thread-safe information about crash traces to the main thread when a thread crashes. + * + * @param mixed[][] $rawTrace + * @phpstan-param list> $rawTrace + * + * @return ThreadCrashInfoFrame[] + */ + public static function printableTraceWithMetadata(array $rawTrace, int $maxStringLength = 80) : array{ + $printableTrace = self::printableTrace($rawTrace, $maxStringLength); + $safeTrace = []; + foreach($printableTrace as $frameId => $printableFrame){ + $rawFrame = $rawTrace[$frameId]; + $safeTrace[$frameId] = new ThreadCrashInfoFrame( + $printableFrame, + $rawFrame["file"] ?? "unknown", + $rawFrame["line"] ?? 0 + ); + } + + return $safeTrace; + } + /** * @return mixed[][] * @phpstan-return list> diff --git a/tests/phpstan/configs/actual-problems.neon b/tests/phpstan/configs/actual-problems.neon index 27e881153..75118cc93 100644 --- a/tests/phpstan/configs/actual-problems.neon +++ b/tests/phpstan/configs/actual-problems.neon @@ -845,6 +845,16 @@ parameters: count: 1 path: ../../../src/utils/Utils.php + - + message: "#^Parameter \\#2 \\$file of class pocketmine\\\\thread\\\\ThreadCrashInfoFrame constructor expects string\\|null, mixed given\\.$#" + count: 1 + path: ../../../src/utils/Utils.php + + - + message: "#^Parameter \\#3 \\$line of class pocketmine\\\\thread\\\\ThreadCrashInfoFrame constructor expects int, mixed given\\.$#" + count: 1 + path: ../../../src/utils/Utils.php + - message: "#^Parameter \\#1 \\$x of method pocketmine\\\\world\\\\World\\:\\:getTileAt\\(\\) expects int, float\\|int given\\.$#" count: 1