diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 835153bb78f..231304a8be5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -58,6 +58,7 @@ PocketMine-MP has three primary branches of development. | Deprecating API classes, methods or constants | ❌ | ✔️ | ✔️ | | Adding optional parameters to an API method | ❌ | ✔️ | ✔️ | | Changing API behaviour | ❌ | 🟡 Only if backwards-compatible | ✔️ | +| Changing an event from sync to async or vice versa | ❌ | ❌ | ✔️ | | Removal of API | ❌ | ❌ | ✔️ | | Backwards-incompatible API change (e.g. renaming a method) | ❌ | ❌ | ✔️ | | Backwards-incompatible internals change (e.g. changing things in `pocketmine\network\mcpe`) | ❌ | ✔️ | ✔️ | diff --git a/src/Server.php b/src/Server.php index 130bf353cb4..61dfa2be6ac 100644 --- a/src/Server.php +++ b/src/Server.php @@ -39,6 +39,7 @@ use pocketmine\data\bedrock\BedrockDataFiles; use pocketmine\entity\EntityDataHelper; use pocketmine\entity\Location; +use pocketmine\event\AsyncHandlerListManager; use pocketmine\event\HandlerListManager; use pocketmine\event\player\PlayerCreationEvent; use pocketmine\event\player\PlayerDataSaveEvent; @@ -1557,6 +1558,7 @@ public function forceShutdown() : void{ $this->logger->debug("Removing event handlers"); HandlerListManager::global()->unregisterAll(); + AsyncHandlerListManager::global()->unregisterAll(); if(isset($this->asyncPool)){ $this->logger->debug("Shutting down async task worker pool"); diff --git a/src/event/AsyncEvent.php b/src/event/AsyncEvent.php new file mode 100644 index 00000000000..a5b398e3f6a --- /dev/null +++ b/src/event/AsyncEvent.php @@ -0,0 +1,136 @@ + $handlersCallState */ + private static array $handlersCallState = []; + private const MAX_CONCURRENT_CALLS = 1000; //max number of concurrent calls to a single handler + + /** + * @phpstan-return Promise + */ + final public function call() : Promise{ + $timings = Timings::getAsyncEventTimings($this); + $timings->startTiming(); + + try{ + /** @phpstan-var PromiseResolver $globalResolver */ + $globalResolver = new PromiseResolver(); + + $handlers = AsyncHandlerListManager::global()->getHandlersFor(static::class); + if(count($handlers) > 0){ + $this->processRemainingHandlers($handlers, fn() => $globalResolver->resolve($this), $globalResolver->reject(...)); + }else{ + $globalResolver->resolve($this); + } + + return $globalResolver->getPromise(); + }finally{ + $timings->stopTiming(); + } + } + + /** + * @param AsyncRegisteredListener[] $handlers + * @phpstan-param array $handlers + * @phpstan-param \Closure() : void $resolve + * @phpstan-param \Closure() : void $reject + */ + private function processRemainingHandlers(array $handlers, \Closure $resolve, \Closure $reject) : void{ + $currentPriority = null; + $awaitPromises = []; + foreach($handlers as $k => $handler){ + $priority = $handler->getPriority(); + if(count($awaitPromises) > 0 && $currentPriority !== null && $currentPriority !== $priority){ + //wait for concurrent promises from previous priority to complete + break; + } + + $currentPriority = $priority; + if(!isset(self::$handlersCallState[$handlerId = spl_object_id($handler)])){ + self::$handlersCallState[$handlerId] = 0; + } + if(self::$handlersCallState[$handlerId] >= self::MAX_CONCURRENT_CALLS){ + throw new \RuntimeException("Concurrent call limit reached for handler " . + Utils::getNiceClosureName($handler->getHandler()) . "(" . Utils::getNiceClassName($this) . ")" . + " (max: " . self::MAX_CONCURRENT_CALLS . ")"); + } + $removeCallback = static function() use ($handlerId) : void{ + --self::$handlersCallState[$handlerId]; + }; + if($handler->canBeCalledConcurrently()){ + unset($handlers[$k]); + ++self::$handlersCallState[$handlerId]; + $promise = $handler->callAsync($this); + if($promise !== null){ + $promise->onCompletion($removeCallback, $removeCallback); + $awaitPromises[] = $promise; + }else{ + $removeCallback(); + } + }else{ + if(count($awaitPromises) > 0){ + //wait for concurrent promises to complete + break; + } + + unset($handlers[$k]); + ++self::$handlersCallState[$handlerId]; + $promise = $handler->callAsync($this); + if($promise !== null){ + $promise->onCompletion($removeCallback, $removeCallback); + $promise->onCompletion( + onSuccess: fn() => $this->processRemainingHandlers($handlers, $resolve, $reject), + onFailure: $reject + ); + return; + } + $removeCallback(); + } + } + + if(count($awaitPromises) > 0){ + Promise::all($awaitPromises)->onCompletion( + onSuccess: fn() => $this->processRemainingHandlers($handlers, $resolve, $reject), + onFailure: $reject + ); + }else{ + $resolve(); + } + } +} diff --git a/src/event/AsyncHandlerListManager.php b/src/event/AsyncHandlerListManager.php new file mode 100644 index 00000000000..1f7be2e5c5c --- /dev/null +++ b/src/event/AsyncHandlerListManager.php @@ -0,0 +1,58 @@ + + */ +final class AsyncHandlerListManager extends BaseHandlerListManager{ + private static ?self $globalInstance = null; + + public static function global() : self{ + return self::$globalInstance ?? (self::$globalInstance = new self()); + } + + protected function getBaseEventClass() : string{ + return AsyncEvent::class; + } + + /** + * @phpstan-param array $listeners + * @phpstan-return array + */ + private static function sortSamePriorityHandlers(array $listeners) : array{ + uasort($listeners, function(AsyncRegisteredListener $left, AsyncRegisteredListener $right) : int{ + //Promise::all() can be used more efficiently if concurrent handlers are grouped together. + //It's not important whether they are grouped before or after exclusive handlers. + return $left->canBeCalledConcurrently() <=> $right->canBeCalledConcurrently(); + }); + return $listeners; + } + + protected function createHandlerList(string $event, ?HandlerList $parentList, RegisteredListenerCache $handlerCache) : HandlerList{ + return new HandlerList($event, $parentList, $handlerCache, self::sortSamePriorityHandlers(...)); + } +} diff --git a/src/event/AsyncRegisteredListener.php b/src/event/AsyncRegisteredListener.php new file mode 100644 index 00000000000..4c5c0b957dc --- /dev/null +++ b/src/event/AsyncRegisteredListener.php @@ -0,0 +1,60 @@ +|null + */ + public function callAsync(AsyncEvent $event) : ?Promise{ + if($event instanceof Cancellable && $event->isCancelled() && !$this->isHandlingCancelled()){ + return null; + } + $this->timings->startTiming(); + try{ + return ($this->handler)($event); + }finally{ + $this->timings->stopTiming(); + } + } + + public function canBeCalledConcurrently() : bool{ + return !$this->exclusiveCall; + } +} diff --git a/src/event/BaseHandlerListManager.php b/src/event/BaseHandlerListManager.php new file mode 100644 index 00000000000..96c6c1fae39 --- /dev/null +++ b/src/event/BaseHandlerListManager.php @@ -0,0 +1,156 @@ + + */ +abstract class BaseHandlerListManager{ + /** + * @var HandlerList[] classname => HandlerList + * @phpstan-var array, THandlerList> + */ + private array $allLists = []; + /** + * @var RegisteredListenerCache[] event class name => cache + * @phpstan-var array, RegisteredListenerCache> + */ + private array $handlerCaches = []; + + /** + * Unregisters all the listeners + * If a Plugin or Listener is passed, all the listeners with that object will be removed + * + * @phpstan-param TRegisteredListener|Plugin|Listener|null $object + */ + public function unregisterAll(BaseRegisteredListener|Plugin|Listener|null $object = null) : void{ + if($object !== null){ + foreach($this->allLists as $h){ + $h->unregister($object); + } + }else{ + foreach($this->allLists as $h){ + $h->clear(); + } + } + } + + /** + * @phpstan-param \ReflectionClass $class + */ + private static function isValidClass(\ReflectionClass $class) : bool{ + $tags = Utils::parseDocComment((string) $class->getDocComment()); + return !$class->isAbstract() || isset($tags["allowHandle"]); + } + + /** + * @phpstan-param \ReflectionClass $class + * + * @phpstan-return \ReflectionClass|null + */ + private static function resolveNearestHandleableParent(\ReflectionClass $class) : ?\ReflectionClass{ + for($parent = $class->getParentClass(); $parent !== false; $parent = $parent->getParentClass()){ + if(self::isValidClass($parent)){ + return $parent; + } + //NOOP + } + return null; + } + + /** + * @phpstan-return class-string + */ + abstract protected function getBaseEventClass() : string; + + /** + * @phpstan-param class-string $event + * @phpstan-param HandlerList|null $parentList + * @phpstan-param RegisteredListenerCache $handlerCache + * + * @phpstan-return THandlerList + */ + abstract protected function createHandlerList(string $event, ?HandlerList $parentList, RegisteredListenerCache $handlerCache) : HandlerList; + + /** + * Returns the HandlerList for listeners that explicitly handle this event. + * + * Calling this method also lazily initializes the $classMap inheritance tree of handler lists. + * + * @phpstan-param class-string $event + * @phpstan-return THandlerList + * + * @throws \ReflectionException + * @throws \InvalidArgumentException + */ + public function getListFor(string $event) : HandlerList{ + if(isset($this->allLists[$event])){ + return $this->allLists[$event]; + } + + $class = new \ReflectionClass($event); + if(!$class->isSubclassOf($this->getBaseEventClass())){ + throw new \InvalidArgumentException("Cannot get sync handler list for async event"); + } + if(!self::isValidClass($class)){ + throw new \InvalidArgumentException("Event must be non-abstract or have the @allowHandle annotation"); + } + + $parent = self::resolveNearestHandleableParent($class); + /** @phpstan-var RegisteredListenerCache $cache */ + $cache = new RegisteredListenerCache(); + $this->handlerCaches[$event] = $cache; + return $this->allLists[$event] = $this->createHandlerList( + $event, + parentList: $parent !== null ? $this->getListFor($parent->getName()) : null, + handlerCache: $cache + ); + } + + /** + * @phpstan-param class-string $event + * + * @return RegisteredListener[] + * @phpstan-return list + */ + public function getHandlersFor(string $event) : array{ + $cache = $this->handlerCaches[$event] ?? null; + //getListFor() will populate the cache for the next call + return $cache->list ?? $this->getListFor($event)->getListenerList(); + } + + /** + * @return HandlerList[] + * @phpstan-return array, THandlerList> + */ + public function getAll() : array{ + return $this->allLists; + } +} diff --git a/src/event/BaseRegisteredListener.php b/src/event/BaseRegisteredListener.php new file mode 100644 index 00000000000..0c685f09b10 --- /dev/null +++ b/src/event/BaseRegisteredListener.php @@ -0,0 +1,58 @@ +handler; + } + + public function getPlugin() : Plugin{ + return $this->plugin; + } + + public function getPriority() : int{ + return $this->priority; + } + + public function isHandlingCancelled() : bool{ + return $this->handleCancelled; + } +} diff --git a/src/event/HandlerList.php b/src/event/HandlerList.php index e7d229a3ffb..b1d53ab8638 100644 --- a/src/event/HandlerList.php +++ b/src/event/HandlerList.php @@ -29,23 +29,33 @@ use function spl_object_id; use const SORT_NUMERIC; +/** + * @phpstan-template TListener of BaseRegisteredListener + */ class HandlerList{ /** - * @var RegisteredListener[][] - * @phpstan-var array> + * @var BaseRegisteredListener[][] + * @phpstan-var array> */ private array $handlerSlots = []; - /** @var RegisteredListenerCache[] */ + /** + * @var RegisteredListenerCache[] + * @phpstan-var array> + */ private array $affectedHandlerCaches = []; /** - * @phpstan-param class-string $class + * @phpstan-param class-string $class + * @phpstan-param ?static $parentList + * @phpstan-param RegisteredListenerCache $handlerCache + * @phpstan-param ?\Closure(array) : array $sortSamePriorityHandlers */ public function __construct( private string $class, private ?HandlerList $parentList, - private RegisteredListenerCache $handlerCache = new RegisteredListenerCache() + private RegisteredListenerCache $handlerCache = new RegisteredListenerCache(), + private ?\Closure $sortSamePriorityHandlers = null ){ for($list = $this; $list !== null; $list = $list->parentList){ $list->affectedHandlerCaches[spl_object_id($this->handlerCache)] = $this->handlerCache; @@ -53,9 +63,9 @@ public function __construct( } /** - * @throws \Exception + * @phpstan-param TListener $listener */ - public function register(RegisteredListener $listener) : void{ + public function register(BaseRegisteredListener $listener) : void{ if(isset($this->handlerSlots[$listener->getPriority()][spl_object_id($listener)])){ throw new \InvalidArgumentException("This listener is already registered to priority {$listener->getPriority()} of event {$this->class}"); } @@ -64,7 +74,8 @@ public function register(RegisteredListener $listener) : void{ } /** - * @param RegisteredListener[] $listeners + * @param BaseRegisteredListener[] $listeners + * @phpstan-param array $listeners */ public function registerAll(array $listeners) : void{ foreach($listeners as $listener){ @@ -73,7 +84,10 @@ public function registerAll(array $listeners) : void{ $this->invalidateAffectedCaches(); } - public function unregister(RegisteredListener|Plugin|Listener $object) : void{ + /** + * @phpstan-param TListener|Plugin|Listener $object + */ + public function unregister(BaseRegisteredListener|Plugin|Listener $object) : void{ if($object instanceof Plugin || $object instanceof Listener){ foreach($this->handlerSlots as $priority => $list){ foreach($list as $hash => $listener){ @@ -96,12 +110,16 @@ public function clear() : void{ } /** - * @return RegisteredListener[] + * @return BaseRegisteredListener[] + * @phpstan-return array */ public function getListenersByPriority(int $priority) : array{ return $this->handlerSlots[$priority] ?? []; } + /** + * @phpstan-return static + */ public function getParent() : ?HandlerList{ return $this->parentList; } @@ -116,8 +134,8 @@ private function invalidateAffectedCaches() : void{ } /** - * @return RegisteredListener[] - * @phpstan-return list + * @return BaseRegisteredListener[] + * @phpstan-return list */ public function getListenerList() : array{ if($this->handlerCache->list !== null){ @@ -132,7 +150,12 @@ public function getListenerList() : array{ $listenersByPriority = []; foreach($handlerLists as $currentList){ foreach($currentList->handlerSlots as $priority => $listeners){ - $listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $listeners); + $listenersByPriority[$priority] = array_merge( + $listenersByPriority[$priority] ?? [], + $this->sortSamePriorityHandlers !== null ? + ($this->sortSamePriorityHandlers)($listeners) : + $listeners + ); } } diff --git a/src/event/HandlerListManager.php b/src/event/HandlerListManager.php index 9437df37f9e..489a3af90a2 100644 --- a/src/event/HandlerListManager.php +++ b/src/event/HandlerListManager.php @@ -23,109 +23,21 @@ namespace pocketmine\event; -use pocketmine\plugin\Plugin; -use pocketmine\utils\Utils; - -class HandlerListManager{ - +/** + * @phpstan-extends BaseHandlerListManager + */ +class HandlerListManager extends BaseHandlerListManager{ private static ?self $globalInstance = null; public static function global() : self{ return self::$globalInstance ?? (self::$globalInstance = new self()); } - /** @var HandlerList[] classname => HandlerList */ - private array $allLists = []; - /** - * @var RegisteredListenerCache[] event class name => cache - * @phpstan-var array, RegisteredListenerCache> - */ - private array $handlerCaches = []; - - /** - * Unregisters all the listeners - * If a Plugin or Listener is passed, all the listeners with that object will be removed - */ - public function unregisterAll(RegisteredListener|Plugin|Listener|null $object = null) : void{ - if($object instanceof Listener || $object instanceof Plugin || $object instanceof RegisteredListener){ - foreach($this->allLists as $h){ - $h->unregister($object); - } - }else{ - foreach($this->allLists as $h){ - $h->clear(); - } - } - } - - /** - * @phpstan-param \ReflectionClass $class - */ - private static function isValidClass(\ReflectionClass $class) : bool{ - $tags = Utils::parseDocComment((string) $class->getDocComment()); - return !$class->isAbstract() || isset($tags["allowHandle"]); - } - - /** - * @phpstan-param \ReflectionClass $class - * - * @phpstan-return \ReflectionClass|null - */ - private static function resolveNearestHandleableParent(\ReflectionClass $class) : ?\ReflectionClass{ - for($parent = $class->getParentClass(); $parent !== false; $parent = $parent->getParentClass()){ - if(self::isValidClass($parent)){ - return $parent; - } - //NOOP - } - return null; - } - - /** - * Returns the HandlerList for listeners that explicitly handle this event. - * - * Calling this method also lazily initializes the $classMap inheritance tree of handler lists. - * - * @phpstan-param class-string $event - * - * @throws \ReflectionException - * @throws \InvalidArgumentException - */ - public function getListFor(string $event) : HandlerList{ - if(isset($this->allLists[$event])){ - return $this->allLists[$event]; - } - - $class = new \ReflectionClass($event); - if(!self::isValidClass($class)){ - throw new \InvalidArgumentException("Event must be non-abstract or have the @allowHandle annotation"); - } - - $parent = self::resolveNearestHandleableParent($class); - $cache = new RegisteredListenerCache(); - $this->handlerCaches[$event] = $cache; - return $this->allLists[$event] = new HandlerList( - $event, - parentList: $parent !== null ? $this->getListFor($parent->getName()) : null, - handlerCache: $cache - ); - } - - /** - * @phpstan-param class-string $event - * - * @return RegisteredListener[] - */ - public function getHandlersFor(string $event) : array{ - $cache = $this->handlerCaches[$event] ?? null; - //getListFor() will populate the cache for the next call - return $cache->list ?? $this->getListFor($event)->getListenerList(); + protected function getBaseEventClass() : string{ + return Event::class; } - /** - * @return HandlerList[] - */ - public function getAll() : array{ - return $this->allLists; + protected function createHandlerList(string $event, ?HandlerList $parentList, RegisteredListenerCache $handlerCache) : HandlerList{ + return new HandlerList($event, $parentList, $handlerCache); } } diff --git a/src/event/ListenerMethodTags.php b/src/event/ListenerMethodTags.php index cb932ce27ac..e65f25f80ba 100644 --- a/src/event/ListenerMethodTags.php +++ b/src/event/ListenerMethodTags.php @@ -31,4 +31,5 @@ final class ListenerMethodTags{ public const HANDLE_CANCELLED = "handleCancelled"; public const NOT_HANDLER = "notHandler"; public const PRIORITY = "priority"; + public const EXCLUSIVE_CALL = "exclusiveCall"; } diff --git a/src/event/RegisteredListener.php b/src/event/RegisteredListener.php index 6b29dfec30f..a2217c2996c 100644 --- a/src/event/RegisteredListener.php +++ b/src/event/RegisteredListener.php @@ -23,34 +23,7 @@ namespace pocketmine\event; -use pocketmine\plugin\Plugin; -use pocketmine\timings\TimingsHandler; -use function in_array; - -class RegisteredListener{ - public function __construct( - private \Closure $handler, - private int $priority, - private Plugin $plugin, - private bool $handleCancelled, - private TimingsHandler $timings - ){ - if(!in_array($priority, EventPriority::ALL, true)){ - throw new \InvalidArgumentException("Invalid priority number $priority"); - } - } - - public function getHandler() : \Closure{ - return $this->handler; - } - - public function getPlugin() : Plugin{ - return $this->plugin; - } - - public function getPriority() : int{ - return $this->priority; - } +class RegisteredListener extends BaseRegisteredListener{ public function callEvent(Event $event) : void{ if($event instanceof Cancellable && $event->isCancelled() && !$this->isHandlingCancelled()){ @@ -63,8 +36,4 @@ public function callEvent(Event $event) : void{ $this->timings->stopTiming(); } } - - public function isHandlingCancelled() : bool{ - return $this->handleCancelled; - } } diff --git a/src/event/RegisteredListenerCache.php b/src/event/RegisteredListenerCache.php index e5e3155465b..ce7bfcb2c48 100644 --- a/src/event/RegisteredListenerCache.php +++ b/src/event/RegisteredListenerCache.php @@ -25,14 +25,14 @@ /** * @internal + * @phpstan-template TListener */ final class RegisteredListenerCache{ /** * List of all handlers that will be called for a particular event, ordered by execution order. * - * @var RegisteredListener[] - * @phpstan-var list + * @phpstan-var list */ public ?array $list = null; } diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index a8440f04fc7..3bb5ebefd86 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -23,6 +23,9 @@ namespace pocketmine\plugin; +use pocketmine\event\AsyncEvent; +use pocketmine\event\AsyncHandlerListManager; +use pocketmine\event\AsyncRegisteredListener; use pocketmine\event\Cancellable; use pocketmine\event\Event; use pocketmine\event\EventPriority; @@ -36,6 +39,7 @@ use pocketmine\permission\DefaultPermissions; use pocketmine\permission\PermissionManager; use pocketmine\permission\PermissionParser; +use pocketmine\promise\Promise; use pocketmine\Server; use pocketmine\timings\Timings; use pocketmine\utils\AssumptionFailedError; @@ -529,6 +533,7 @@ public function disablePlugin(Plugin $plugin) : void{ $plugin->onEnableStateChange(false); $plugin->getScheduler()->shutdown(); HandlerListManager::global()->unregisterAll($plugin); + AsyncHandlerListManager::global()->unregisterAll($plugin); } } @@ -582,7 +587,7 @@ private function getEventsHandledBy(\ReflectionMethod $method) : ?string{ /** @phpstan-var class-string $paramClass */ $paramClass = $paramType->getName(); $eventClass = new \ReflectionClass($paramClass); - if(!$eventClass->isSubclassOf(Event::class)){ + if(!$eventClass->isSubclassOf(Event::class) && !$eventClass->isSubclassOf(AsyncEvent::class)){ return null; } @@ -636,8 +641,36 @@ public function registerEvents(Listener $listener, Plugin $plugin) : void{ throw new PluginException("Event handler " . Utils::getNiceClosureName($handlerClosure) . "() declares invalid @" . ListenerMethodTags::HANDLE_CANCELLED . " value \"" . $tags[ListenerMethodTags::HANDLE_CANCELLED] . "\""); } } + $exclusiveCall = false; + if(isset($tags[ListenerMethodTags::EXCLUSIVE_CALL])){ + if(!is_a($eventClass, AsyncEvent::class, true)){ + throw new PluginException(sprintf( + "Event handler %s() declares @%s for non-async event of type %s", + Utils::getNiceClosureName($handlerClosure), + ListenerMethodTags::EXCLUSIVE_CALL, + $eventClass + )); + } + switch(strtolower($tags[ListenerMethodTags::EXCLUSIVE_CALL])){ + case "true": + case "": + $exclusiveCall = true; + break; + case "false": + break; + default: + throw new PluginException("Event handler " . Utils::getNiceClosureName($handlerClosure) . "() declares invalid @" . ListenerMethodTags::EXCLUSIVE_CALL . " value \"" . $tags[ListenerMethodTags::EXCLUSIVE_CALL] . "\""); + } + } - $this->registerEvent($eventClass, $handlerClosure, $priority, $plugin, $handleCancelled); + if(is_subclass_of($eventClass, AsyncEvent::class)){ + if(!$this->canHandleAsyncEvent($handlerClosure)){ + throw new PluginException("Event handler " . Utils::getNiceClosureName($handlerClosure) . " must return null|Promise to be able to handle async events"); + } + $this->registerAsyncEvent($eventClass, $handlerClosure, $priority, $plugin, $handleCancelled, $exclusiveCall); + }else{ + $this->registerEvent($eventClass, $handlerClosure, $priority, $plugin, $handleCancelled); + } } } @@ -672,4 +705,46 @@ public function registerEvent(string $event, \Closure $handler, int $priority, P HandlerListManager::global()->getListFor($event)->register($registeredListener); return $registeredListener; } + + /** + * @param string $event Class name that extends Event and AsyncEvent + * + * @phpstan-template TEvent of AsyncEvent + * @phpstan-param class-string $event + * @phpstan-param \Closure(TEvent) : ?Promise $handler + * + * @throws \ReflectionException + */ + public function registerAsyncEvent(string $event, \Closure $handler, int $priority, Plugin $plugin, bool $handleCancelled = false, bool $exclusiveCall = false) : AsyncRegisteredListener{ + //TODO: Not loving the code duplication here + if(!is_subclass_of($event, AsyncEvent::class)){ + throw new PluginException($event . " is not an AsyncEvent"); + } + + $handlerName = Utils::getNiceClosureName($handler); + + if(!$plugin->isEnabled()){ + throw new PluginException("Plugin attempted to register event handler " . $handlerName . "() to event " . $event . " while not enabled"); + } + + $timings = Timings::getEventHandlerTimings($event, $handlerName, $plugin->getDescription()->getFullName()); + + $registeredListener = new AsyncRegisteredListener($handler, $priority, $plugin, $handleCancelled, $exclusiveCall, $timings); + AsyncHandlerListManager::global()->getListFor($event)->register($registeredListener); + return $registeredListener; + } + + /** + * Check if the given handler return type is async-compatible (equal to Promise) + * + * @phpstan-param \Closure(AsyncEvent) : Promise $handler + * + * @throws \ReflectionException + */ + private function canHandleAsyncEvent(\Closure $handler) : bool{ + $reflection = new \ReflectionFunction($handler); + $return = $reflection->getReturnType(); + + return $return instanceof \ReflectionNamedType && $return->getName() === Promise::class; + } } diff --git a/src/timings/Timings.php b/src/timings/Timings.php index ee737f537b0..956de6cf966 100644 --- a/src/timings/Timings.php +++ b/src/timings/Timings.php @@ -25,6 +25,7 @@ use pocketmine\block\tile\Tile; use pocketmine\entity\Entity; +use pocketmine\event\AsyncEvent; use pocketmine\event\Event; use pocketmine\network\mcpe\protocol\ClientboundPacket; use pocketmine\network\mcpe\protocol\ServerboundPacket; @@ -116,6 +117,8 @@ abstract class Timings{ /** @var TimingsHandler[] */ private static array $events = []; + /** @var TimingsHandler[] */ + private static array $asyncEvents = []; /** @var TimingsHandler[][] */ private static array $eventHandlers = []; @@ -313,8 +316,18 @@ public static function getEventTimings(Event $event) : TimingsHandler{ return self::$events[$eventClass]; } + public static function getAsyncEventTimings(AsyncEvent $event) : TimingsHandler{ + $eventClass = get_class($event); + if(!isset(self::$asyncEvents[$eventClass])){ + self::$asyncEvents[$eventClass] = new TimingsHandler(self::shortenCoreClassName($eventClass, "pocketmine\\event\\"), group: "Events"); + } + + return self::$asyncEvents[$eventClass]; + } + /** - * @phpstan-param class-string $event + * @phpstan-template TEvent of Event|AsyncEvent + * @phpstan-param class-string $event */ public static function getEventHandlerTimings(string $event, string $handlerName, string $group) : TimingsHandler{ if(!isset(self::$eventHandlers[$event][$handlerName])){ diff --git a/tests/phpstan/configs/impossible-generics.neon b/tests/phpstan/configs/impossible-generics.neon index e0b944e695d..78ea415e199 100644 --- a/tests/phpstan/configs/impossible-generics.neon +++ b/tests/phpstan/configs/impossible-generics.neon @@ -4,11 +4,41 @@ parameters: message: '#^Method pocketmine\\event\\RegisteredListener\:\:__construct\(\) has parameter \$handler with no signature specified for Closure\.$#' identifier: missingType.callable count: 1 - path: ../../../src/event/RegisteredListener.php + path: ../../../src/event/AsyncRegisteredListener.php - message: '#^Method pocketmine\\event\\RegisteredListener\:\:getHandler\(\) return type has no signature specified for Closure\.$#' identifier: missingType.callable count: 1 - path: ../../../src/event/RegisteredListener.php + path: ../../../src/event/BaseRegisteredListener.php + + - + message: "#^Method pocketmine\\\\event\\\\BaseRegisteredListener\\:\\:getHandler\\(\\) return type has no signature specified for Closure\\.$#" + identifier: missingType.callable + count: 1 + path: ../../../src/event/BaseRegisteredListener.php + + - + message: '#^Parameter \#2 \$parentList of class pocketmine\\event\\HandlerList constructor expects static\(pocketmine\\event\\HandlerList\\)\|null, pocketmine\\event\\HandlerList\\|null given\.$#' + identifier: argument.type + count: 1 + path: ../../../src/event/AsyncHandlerListManager.php + + - + message: '#^Method pocketmine\\event\\AsyncRegisteredListener\:\:__construct\(\) has parameter \$handler with no signature specified for Closure\.$#' + identifier: missingType.callable + count: 1 + path: ../../../src/event/AsyncRegisteredListener.php + + - + message: '#^Method pocketmine\\event\\BaseRegisteredListener\:\:__construct\(\) has parameter \$handler with no signature specified for Closure\.$#' + identifier: missingType.callable + count: 1 + path: ../../../src/event/BaseRegisteredListener.php + + - + message: '#^Parameter \#2 \$parentList of class pocketmine\\event\\HandlerList constructor expects static\(pocketmine\\event\\HandlerList\\)\|null, pocketmine\\event\\HandlerList\\|null given\.$#' + identifier: argument.type + count: 1 + path: ../../../src/event/HandlerListManager.php diff --git a/tests/phpunit/event/AsyncEventConcurrencyTest.php b/tests/phpunit/event/AsyncEventConcurrencyTest.php new file mode 100644 index 00000000000..b20e4987526 --- /dev/null +++ b/tests/phpunit/event/AsyncEventConcurrencyTest.php @@ -0,0 +1,125 @@ +> + */ + private array $resolvers = []; + + private bool $activeExclusiveHandler = false; + private bool $activeConcurrentHandler = false; + + private int $done = 0; + + protected function setUp() : void{ + AsyncHandlerListManager::global()->unregisterAll(); + + //TODO: this is a really bad hack and could break any time if PluginManager decides to access its Server field + //we really need to make it possible to register events without a Plugin or Server context + $mockServer = $this->createMock(Server::class); + $this->mockPlugin = self::createStub(Plugin::class); + $this->mockPlugin->method('isEnabled')->willReturn(true); + $this->pluginManager = new PluginManager($mockServer, null); + } + + public static function tearDownAfterClass() : void{ + AsyncHandlerListManager::global()->unregisterAll(); + } + + /** + * @phpstan-return Promise + */ + private function handler(bool &$flag, string $label) : Promise{ + $flag = true; + /** @var PromiseResolver $resolver */ + $resolver = new PromiseResolver(); + $this->resolvers[] = $resolver; + $resolver->getPromise()->onCompletion( + function() use (&$flag) : void{ + $flag = false; + $this->done++; + }, + fn() => self::fail("Not expecting this to be rejected for $label") + ); + return $resolver->getPromise(); + } + + public function testConcurrency() : void{ + $this->pluginManager->registerAsyncEvent( + TestGrandchildAsyncEvent::class, + function(TestGrandchildAsyncEvent $event) : Promise{ + self::assertFalse($this->activeExclusiveHandler, "Concurrent handler can't run while exclusive handlers are waiting to complete"); + + return $this->handler($this->activeConcurrentHandler, "concurrent"); + }, + EventPriority::NORMAL, + $this->mockPlugin, + //non-exclusive - this must be completed before any exclusive handlers are run (or run after them) + ); + for($i = 0; $i < 2; $i++){ + $this->pluginManager->registerAsyncEvent( + TestGrandchildAsyncEvent::class, + function(TestGrandchildAsyncEvent $event) use ($i) : Promise{ + self::assertFalse($this->activeExclusiveHandler, "Exclusive handler $i can't run alongside other exclusive handlers"); + self::assertFalse($this->activeConcurrentHandler, "Exclusive handler $i can't run alongside concurrent handler"); + + return $this->handler($this->activeExclusiveHandler, "exclusive $i"); + }, + EventPriority::NORMAL, + $this->mockPlugin, + exclusiveCall: true + ); + } + + (new TestGrandchildAsyncEvent())->call(); + + while(count($this->resolvers) > 0 && $this->done < 3){ + foreach($this->resolvers as $k => $resolver){ + unset($this->resolvers[$k]); + //don't clear the array here - resolving this will trigger adding the next resolver + $resolver->resolve(null); + } + } + + self::assertSame(3, $this->done, "Expected feedback from exactly 3 handlers"); + } +} diff --git a/tests/phpunit/event/AsyncEventTest.php b/tests/phpunit/event/AsyncEventTest.php new file mode 100644 index 00000000000..32dc3a0326a --- /dev/null +++ b/tests/phpunit/event/AsyncEventTest.php @@ -0,0 +1,130 @@ +unregisterAll(); + + //TODO: this is a really bad hack and could break any time if PluginManager decides to access its Server field + //we really need to make it possible to register events without a Plugin or Server context + $mockServer = $this->createMock(Server::class); + $this->mockPlugin = self::createStub(Plugin::class); + $this->mockPlugin->method('isEnabled')->willReturn(true); + $this->pluginManager = new PluginManager($mockServer, null); + } + + public static function tearDownAfterClass() : void{ + AsyncHandlerListManager::global()->unregisterAll(); + } + + public function testHandlerInheritance() : void{ + $expectedOrder = [ + TestGrandchildAsyncEvent::class, + TestChildAsyncEvent::class, + TestParentAsyncEvent::class + ]; + $classes = $expectedOrder; + $actualOrder = []; + shuffle($classes); + foreach($classes as $class){ + $this->pluginManager->registerAsyncEvent( + $class, + function(AsyncEvent $event) use (&$actualOrder, $class) : ?Promise{ + $actualOrder[] = $class; + return null; + }, + EventPriority::NORMAL, + $this->mockPlugin + ); + } + + $event = new TestGrandchildAsyncEvent(); + $promise = $event->call(); + + $resolved = false; + $promise->onCompletion( + function() use ($expectedOrder, $actualOrder, &$resolved){ + self::assertSame($expectedOrder, $actualOrder, "Expected event handlers to be called from most specific to least specific"); + $resolved = true; + }, + fn() => self::fail("Not expecting this to be rejected") + ); + + self::assertTrue($resolved, "No promises were used, expected this promise to resolve immediately"); + } + + public function testPriorityLock() : void{ + $resolver = null; + $firstCompleted = false; + $run = 0; + + $this->pluginManager->registerAsyncEvent( + TestGrandchildAsyncEvent::class, + function(TestGrandchildAsyncEvent $event) use (&$resolver, &$firstCompleted, &$run) : Promise{ + $run++; + /** @var PromiseResolver $resolver */ + $resolver = new PromiseResolver(); + + $resolver->getPromise()->onCompletion( + function() use (&$firstCompleted) : void{ $firstCompleted = true; }, + fn() => self::fail("Not expecting this to be rejected") + ); + + return $resolver->getPromise(); + }, + EventPriority::LOW, //anything below NORMAL is fine + $this->mockPlugin + ); + $this->pluginManager->registerAsyncEvent( + TestGrandchildAsyncEvent::class, + function(TestGrandchildAsyncEvent $event) use (&$firstCompleted, &$run) : ?Promise{ + $run++; + self::assertTrue($firstCompleted, "This shouldn't run until the previous priority is done"); + return null; + }, + EventPriority::NORMAL, + $this->mockPlugin + ); + + (new TestGrandchildAsyncEvent())->call(); + self::assertNotNull($resolver, "First handler didn't provide a resolver"); + $resolver->resolve(null); + self::assertSame(2, $run, "Expected feedback from 2 handlers"); + } +} diff --git a/tests/phpunit/event/ConcurrentAsyncEventLimiterTest.php b/tests/phpunit/event/ConcurrentAsyncEventLimiterTest.php new file mode 100644 index 00000000000..288cd886df9 --- /dev/null +++ b/tests/phpunit/event/ConcurrentAsyncEventLimiterTest.php @@ -0,0 +1,151 @@ +unregisterAll(); + + //TODO: this is a really bad hack and could break any time if PluginManager decides to access its Server field + //we really need to make it possible to register events without a Plugin or Server context + $mockServer = $this->createMock(Server::class); + $this->mockPlugin = self::createStub(Plugin::class); + $this->mockPlugin->method('isEnabled')->willReturn(true); + $this->pluginManager = new PluginManager($mockServer, null); + } + + public function tearDown() : void{ + AsyncHandlerListManager::global()->unregisterAll(); + } + + public function testMaxConcurrentCallsNotBlocking() : void{ + self::expectNotToPerformAssertions(); + $this->pluginManager->registerAsyncEvent( + TestParentAsyncEvent::class, + function(TestParentAsyncEvent $e) : ?Promise{ + return null; + }, + EventPriority::NORMAL, + $this->mockPlugin, + ); + + for($i = 0; $i < $this->getMaxConcurrentCalls() + 1; $i++){ + (new TestParentAsyncEvent())->call(); + } + } + + public function testMaxConcurrentCallsBlocking() : void{ + $this->expectConcurrentCallLimitReached(); + $this->pluginManager->registerAsyncEvent( + TestParentAsyncEvent::class, + function(TestParentAsyncEvent $e) : Promise{ + /** @var PromiseResolver $resolver */ + $resolver = new PromiseResolver(); + return $resolver->getPromise(); + }, + EventPriority::NORMAL, + $this->mockPlugin, + ); + + for($i = 0; $i < $this->getMaxConcurrentCalls() + 1; $i++){ + (new TestParentAsyncEvent())->call(); + } + } + + public function testMaxConcurrentCallsDifferentHandlers() : void{ + $resolvers = []; + $this->pluginManager->registerAsyncEvent( + TestParentAsyncEvent::class, + function(TestParentAsyncEvent $e) use (&$resolvers) : ?Promise{ + if($this->handlerA){ + return null; + } + /** @var PromiseResolver $resolver */ + $resolver = new PromiseResolver(); + $resolvers[] = $resolver; + return $resolver->getPromise(); + }, + EventPriority::NORMAL, + $this->mockPlugin, + ); + $this->pluginManager->registerAsyncEvent( + TestParentAsyncEvent::class, + function(TestParentAsyncEvent $e) use (&$resolvers) : ?Promise{ + if(!$this->handlerA){ + return null; + } + /** @var PromiseResolver $resolver */ + $resolver = new PromiseResolver(); + $resolvers[] = $resolver; + return $resolver->getPromise(); + }, + EventPriority::NORMAL, + $this->mockPlugin, + ); + + for($i = 0; $i < $this->getMaxConcurrentCalls() + 1; $i++){ + $this->handlerA = !$this->handlerA; + (new TestParentAsyncEvent())->call(); + } + self::assertCount($this->getMaxConcurrentCalls() + 1, $resolvers, "Expected all promises to be pending"); + + //resolve all the pending promises to clear concurrent calls stack + foreach($resolvers as $resolver){ + $resolver->resolve(null); + } + $resolvers = []; + + $this->expectConcurrentCallLimitReached(); + for($i = 0; $i < ($this->getMaxConcurrentCalls() * 2) + 1; $i++){ + $this->handlerA = !$this->handlerA; + (new TestParentAsyncEvent())->call(); + } + } + + private function getMaxConcurrentCalls() : int{ + $refClass = new \ReflectionClass(AsyncEvent::class); + $value = $refClass->getConstant('MAX_CONCURRENT_CALLS'); + return is_int($value) ? $value : throw new \AssertionError("Max concurrent calls should be an integer"); + } + + private function expectConcurrentCallLimitReached() : void{ + self::expectException(\RuntimeException::class); + self::expectExceptionMessageMatches("/Concurrent call limit reached/"); + } +} diff --git a/tests/phpunit/event/fixtures/TestChildAsyncEvent.php b/tests/phpunit/event/fixtures/TestChildAsyncEvent.php new file mode 100644 index 00000000000..4b19d31d180 --- /dev/null +++ b/tests/phpunit/event/fixtures/TestChildAsyncEvent.php @@ -0,0 +1,28 @@ +