Poll console on the main thread, instead of using a separate thread

There's no need to use an extra thread for this, since there's no concern of a socket getting stuck in a blocking read.

This is one less thing that can go wrong because of pthreads.
This commit is contained in:
Dylan K. Taylor 2022-09-02 00:58:49 +01:00
parent 2585160ca2
commit b3f03d7ae6
No known key found for this signature in database
GPG Key ID: 8927471A91CAFD3D
2 changed files with 68 additions and 71 deletions

View File

@ -31,7 +31,7 @@ use pocketmine\command\Command;
use pocketmine\command\CommandSender;
use pocketmine\command\SimpleCommandMap;
use pocketmine\console\ConsoleCommandSender;
use pocketmine\console\ConsoleReaderThread;
use pocketmine\console\ConsoleReaderChildProcessDaemon;
use pocketmine\crafting\CraftingManager;
use pocketmine\crafting\CraftingManagerFromDataHelper;
use pocketmine\crash\CrashDump;
@ -225,7 +225,8 @@ class Server{
private MemoryManager $memoryManager;
private ?ConsoleReaderThread $console = null;
private ?ConsoleReaderChildProcessDaemon $console = null;
private ?ConsoleCommandSender $consoleSender = null;
private SimpleCommandMap $commandMap;
@ -1049,17 +1050,7 @@ class Server{
$this->subscribeToBroadcastChannel(self::BROADCAST_CHANNEL_USERS, $consoleSender);
if($this->configGroup->getPropertyBool("console.enable-input", true)){
$consoleNotifier = new SleeperNotifier();
$commandBuffer = new \Threaded();
$this->console = new ConsoleReaderThread($commandBuffer, $consoleNotifier);
$this->tickSleeper->addNotifier($consoleNotifier, function() use ($commandBuffer, $consoleSender) : void{
Timings::$serverCommand->startTiming();
while(($line = $commandBuffer->shift()) !== null){
$this->dispatchCommand($consoleSender, (string) $line);
}
Timings::$serverCommand->stopTiming();
});
$this->console->start(PTHREADS_INHERIT_NONE);
$this->console = new ConsoleReaderChildProcessDaemon($this->logger);
}
$this->tickProcessor();
@ -1855,6 +1846,13 @@ class Server{
$this->getMemoryManager()->check();
if($this->console !== null){
while(($line = $this->console->readLine()) !== null){
$this->consoleSender ??= new ConsoleCommandSender($this, $this->language);
$this->dispatchCommand($this->consoleSender, $line);
}
}
Timings::$serverTick->stopTiming();
$now = microtime(true);

View File

@ -23,8 +23,6 @@ declare(strict_types=1);
namespace pocketmine\console;
use pocketmine\snooze\SleeperNotifier;
use pocketmine\thread\Thread;
use pocketmine\utils\AssumptionFailedError;
use pocketmine\utils\Utils;
use Webmozart\PathUtil\Path;
@ -45,36 +43,35 @@ use function trim;
use const PHP_BINARY;
use const STREAM_SHUT_RDWR;
final class ConsoleReaderThread extends Thread{
/**
* This pile of shit exists because PHP on Windows is broken, and can't handle stream_select() on stdin or pipes
* properly - stdin native triggers stream_select() when a key is pressed, causing it to get stuck in fgets()
* waiting for a line that might never come (and Windows doesn't support character-based reading either), and
* pipes just constantly trigger stream_select() instead of only when data is returned, rendering it useless.
*
* This results in whichever process reads stdin getting stuck on shutdown, which previously forced us to kill
* the entire server process to make it go away.
*
* To get around this problem, we delegate the responsibility of reading stdin to a subprocess, which we can
* then brutally murder when the server shuts down, without killing the entire server process.
* Thankfully, stream_select() actually works properly on sockets, so we can use them for inter-process
* communication.
*/
final class ConsoleReaderChildProcessDaemon{
private \PrefixedLogger $logger;
/** @var resource */
private $subprocess;
/** @var resource */
private $socket;
public function __construct(
private \Threaded $buffer,
private ?SleeperNotifier $notifier = null
){}
protected function onRun() : void{
$buffer = $this->buffer;
$notifier = $this->notifier;
while(!$this->isKilled){
$this->runSubprocess($buffer, $notifier);
}
\Logger $logger
){
$this->logger = new \PrefixedLogger($logger, "Console Reader Daemon");
$this->prepareSubprocess();
}
/**
* This pile of shit exists because PHP on Windows is broken, and can't handle stream_select() on stdin or pipes
* properly - stdin native triggers stream_select() when a key is pressed, causing it to get stuck in fgets()
* waiting for a line that might never come (and Windows doesn't support character-based reading either), and
* pipes just constantly trigger stream_select() instead of only when data is returned, rendering it useless.
*
* This results in whichever process reads stdin getting stuck on shutdown, which previously forced us to kill
* the entire server process to make it go away.
*
* To get around this problem, we delegate the responsibility of reading stdin to a subprocess, which we can
* then brutally murder when the server shuts down, without killing the entire server process.
* Thankfully, stream_select() actually works properly on sockets, so we can use them for inter-process
* communication.
*/
private function runSubprocess(\Threaded $buffer, ?SleeperNotifier $notifier) : void{
private function prepareSubprocess() : void{
$server = stream_socket_server("tcp://127.0.0.1:0");
if($server === false){
throw new \RuntimeException("Failed to open console reader socket server");
@ -96,41 +93,43 @@ final class ConsoleReaderThread extends Thread{
throw new AssumptionFailedError("stream_socket_accept() returned false");
}
stream_socket_shutdown($server, STREAM_SHUT_RDWR);
while(!$this->isKilled){
$r = [$client];
$w = null;
$e = null;
if(stream_select($r, $w, $e, 0, 200000) === 1){
$command = fgets($client);
if($command === false){
//subprocess died for some reason; this could be someone killed it manually from outside (e.g.
//mistyped PID) or it might be a ctrl+c signal to this process that the child is handling
//differently (different signal handlers).
//since we have no way to know the difference, we just kill the sub and start a new one.
break;
}
$command = preg_replace("#\\x1b\\x5b([^\\x1b]*\\x7e|[\\x40-\\x50])#", "", trim($command)) ?? throw new AssumptionFailedError("This regex is assumed to be valid");
$command = preg_replace('/[[:cntrl:]]/', '', $command) ?? throw new AssumptionFailedError("This regex is assumed to be valid");
if($command === ""){
continue;
}
$buffer[] = $command;
if($notifier !== null){
$notifier->wakeupSleeper();
}
}
}
$this->subprocess = $sub;
$this->socket = $client;
}
private function shutdownSubprocess() : void{
//we have no way to signal to the subprocess to shut down gracefully; besides, Windows sucks, and the subprocess
//gets stuck in a blocking fgets() read because stream_select() is a hunk of junk (hence the separate process in
//the first place).
proc_terminate($sub);
proc_close($sub);
stream_socket_shutdown($client, STREAM_SHUT_RDWR);
proc_terminate($this->subprocess);
proc_close($this->subprocess);
stream_socket_shutdown($this->socket, STREAM_SHUT_RDWR);
}
public function getThreadName() : string{
return "Console";
public function readLine() : ?string{
$r = [$this->socket];
$w = null;
$e = null;
if(stream_select($r, $w, $e, 0, 0) === 1){
$command = fgets($this->socket);
if($command === false){
$this->logger->debug("Lost connection to subprocess, restarting (maybe the child process was killed from outside?)");
$this->shutdownSubprocess();
$this->prepareSubprocess();
return null;
}
$command = preg_replace("#\\x1b\\x5b([^\\x1b]*\\x7e|[\\x40-\\x50])#", "", trim($command)) ?? throw new AssumptionFailedError("This regex is assumed to be valid");
$command = preg_replace('/[[:cntrl:]]/', '', $command) ?? throw new AssumptionFailedError("This regex is assumed to be valid");
return $command !== "" ? $command : null;
}
return null;
}
public function quit() : void{
$this->shutdownSubprocess();
}
}