Skip to content
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

Generalize asynchronous events #6092

Open
wants to merge 56 commits into
base: minor-next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 54 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
5fe57a8
temporaly add Promise::all
ShockedPlot7560 Oct 14, 2023
a84fc2b
introduce AsyncEvent and ::callAsync()
ShockedPlot7560 Oct 14, 2023
7a4b9a0
events: asynchandler is defined by their return type and event type
ShockedPlot7560 Oct 22, 2023
9b2b92a
oops, remove test code
ShockedPlot7560 Oct 22, 2023
b78ff00
fix style
ShockedPlot7560 Oct 22, 2023
c250bb0
undo Promise covariant + improve array types
ShockedPlot7560 Oct 22, 2023
58155a7
fix PHPstan
ShockedPlot7560 Oct 22, 2023
2b2fa9d
phpstan: populate baseline
ShockedPlot7560 Oct 22, 2023
1176b70
Update src/player/Player.php
dktapps Oct 23, 2023
dc85bba
merge remote tracking
ShockedPlot7560 Oct 27, 2023
ed739cf
cannot call async event in sync context + remove Event dependency for…
ShockedPlot7560 Oct 27, 2023
7e87fbb
clarifying the exception message
ShockedPlot7560 Oct 27, 2023
5beaa3c
correction of various problems
ShockedPlot7560 Oct 27, 2023
cc6e8ef
move the asynchronous registration of handlers to a dedicated PluginM…
ShockedPlot7560 Oct 27, 2023
ca95b2f
fix PHPStan
ShockedPlot7560 Oct 27, 2023
823d4ea
inconsistency correction
ShockedPlot7560 Oct 27, 2023
243a303
follow up of #6110
ShockedPlot7560 Oct 27, 2023
aaa37ba
handlerListe: reduce code complexity
ShockedPlot7560 Oct 27, 2023
f82c422
remove using of Event API
ShockedPlot7560 Jan 21, 2024
64bbff6
Merge remote-tracking branch 'upstream/minor-next' into feat/async-ev…
ShockedPlot7560 Jan 21, 2024
d6b7a9e
merge remote tracking upstream
ShockedPlot7560 Jan 21, 2024
eb98141
resolve AsyncEvent with self instance
ShockedPlot7560 Jan 21, 2024
c1e3903
fix PHPstan
ShockedPlot7560 Jan 21, 2024
b276133
Merge remote-tracking branch 'origin/minor-next' into feat/async-events
ShockedPlot7560 Jul 2, 2024
86fb041
Merge branch 'minor-next' of github.com:pmmp/PocketMine-MP into feat/…
dktapps Nov 13, 2024
b82d47d
Merge branch 'minor-next' into feat/async-events
dktapps Nov 13, 2024
48d2430
Update PHPStan baseline
dktapps Nov 13, 2024
8f48fe4
Fully separate hierarchies for sync & async events
dktapps Nov 13, 2024
17ae932
HandlerListManager: added getter
dktapps Nov 13, 2024
c426677
optimization
dktapps Nov 13, 2024
db88e54
Fix PHPStan error
dktapps Nov 13, 2024
a14afb4
Add integration tests
dktapps Nov 13, 2024
cb2fade
Fixed bug in concurrency integration test
dktapps Nov 13, 2024
409066c
AsyncEvent: make the code easier to make sense of
dktapps Nov 13, 2024
a6a44bd
Fix doc comments
dktapps Nov 13, 2024
6f40c6f
CS
dktapps Nov 13, 2024
32b1d6c
Fixed test code
dktapps Nov 13, 2024
fa79653
ah hello my old friend, impossible-generics.neon
dktapps Nov 13, 2024
8aed5d6
Handler inheritance is now working
dktapps Nov 13, 2024
96989d1
cleanup
dktapps Nov 13, 2024
ac1cf73
Reduce code duplication
dktapps Nov 13, 2024
972a9fb
PluginManager: ensure that handler candidates of async events with wr…
dktapps Nov 13, 2024
667656b
Split AsyncHandlerListManager
dktapps Nov 13, 2024
edae9f2
Reduce number of classes
dktapps Nov 13, 2024
11fdf79
...
dktapps Nov 13, 2024
0a56cf8
Remove unused class
dktapps Nov 13, 2024
a7a1077
CONTRIBUTING: changing an event from sync to async or vice versa is a…
dktapps Nov 13, 2024
117026c
Merge branch 'minor-next' into feat/async-events
dktapps Nov 13, 2024
d2d663b
Simplify handler sorting
dktapps Nov 14, 2024
4451770
Merge branch 'minor-next' into feat/async-events
dktapps Nov 20, 2024
406e2c6
Convert integration tests to unit tests
dktapps Nov 20, 2024
d9f5634
CS
dktapps Nov 20, 2024
d9080f1
we don't need a fake server instance outside of setUp() in these tests
dktapps Nov 20, 2024
866d473
Merge branch 'minor-next' into feat/async-events
dktapps Nov 29, 2024
e8ec81d
fix PHPStan error
dktapps Nov 29, 2024
a0d69a9
github web editor don't fuck up indentation, challenge impossible
dktapps Nov 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,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`) | ❌ | ✔️ | ✔️ |
Expand Down
2 changes: 2 additions & 0 deletions src/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
use pocketmine\crash\CrashDumpRenderer;
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;
Expand Down Expand Up @@ -1492,6 +1493,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");
Expand Down
126 changes: 126 additions & 0 deletions src/event/AsyncEvent.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
<?php

/*
*
* ____ _ _ __ __ _ __ __ ____
* | _ \ ___ ___| | _____| |_| \/ (_)_ __ ___ | \/ | _ \
* | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
* | __/ (_) | (__| < __/ |_| | | | | | | | __/_____| | | | __/
* |_| \___/ \___|_|\_\___|\__|_| |_|_|_| |_|\___| |_| |_|_|
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* @author PocketMine Team
* @link http://www.pocketmine.net/
*
*
*/

declare(strict_types=1);

namespace pocketmine\event;

use pocketmine\promise\Promise;
use pocketmine\promise\PromiseResolver;
use pocketmine\timings\Timings;
use function count;

/**
* This class is used to permit asynchronous event handling.
*
* When an event is called asynchronously, the event handlers are called by priority level.
* When all the promises of a priority level have been resolved, the next priority level is called.
*/
abstract class AsyncEvent{
/** @var array<class-string<AsyncEvent>, int> $delegatesCallDepth */
private static array $delegatesCallDepth = [];
private const MAX_EVENT_CALL_DEPTH = 50;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if I dispatch 50 AsyncPlayerLoginEvents within 1 second when server starts, and the handlers take more than 1 second to resolve? The server crashes immediately?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I discussed this with Plot on Discord yesterday. I don't think even he knows how this code was supposed to work.


/**
* @phpstan-return Promise<static>
*/
final public function call() : Promise{
if(!isset(self::$delegatesCallDepth[$class = static::class])){
self::$delegatesCallDepth[$class] = 0;
}

if(self::$delegatesCallDepth[$class] >= self::MAX_EVENT_CALL_DEPTH){
//this exception will be caught by the parent event call if all else fails
throw new \RuntimeException("Recursive event call detected (reached max depth of " . self::MAX_EVENT_CALL_DEPTH . " calls)");
}

$timings = Timings::getAsyncEventTimings($this);
$timings->startTiming();

++self::$delegatesCallDepth[$class];
try{
/** @phpstan-var PromiseResolver<static> $globalResolver */
$globalResolver = new PromiseResolver();

$handlers = AsyncHandlerListManager::global()->getHandlersFor(static::class);
if(count($handlers) > 0){
dktapps marked this conversation as resolved.
Show resolved Hide resolved
$this->processRemainingHandlers($handlers, fn() => $globalResolver->resolve($this), $globalResolver->reject(...));
}else{
$globalResolver->resolve($this);
}

return $globalResolver->getPromise();
}finally{
--self::$delegatesCallDepth[$class];
$timings->stopTiming();
}
}

/**
* @param AsyncRegisteredListener[] $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){

Check failure on line 85 in src/event/AsyncEvent.php

View workflow job for this annotation

GitHub Actions / PHP 8.1 / PHPStan analysis

Unsafe foreach on array with key type (int|string) (they might be casted to int).

Check failure on line 85 in src/event/AsyncEvent.php

View workflow job for this annotation

GitHub Actions / PHP 8.2 / PHPStan analysis

Unsafe foreach on array with key type (int|string) (they might be casted to int).

Check failure on line 85 in src/event/AsyncEvent.php

View workflow job for this annotation

GitHub Actions / PHP 8.1 / PHPStan analysis

Unsafe foreach on array with key type (int|string) (they might be casted to int).

Check failure on line 85 in src/event/AsyncEvent.php

View workflow job for this annotation

GitHub Actions / PHP 8.2 / PHPStan analysis

Unsafe foreach on array with key type (int|string) (they might be casted to int).

Check failure on line 85 in src/event/AsyncEvent.php

View workflow job for this annotation

GitHub Actions / PHP 8.3 / PHPStan analysis

Unsafe foreach on array with key type (int|string) (they might be casted to int).

Check failure on line 85 in src/event/AsyncEvent.php

View workflow job for this annotation

GitHub Actions / PHP 8.3 / PHPStan analysis

Unsafe foreach on array with key type (int|string) (they might be casted to int).
dktapps marked this conversation as resolved.
Show resolved Hide resolved
$priority = $handler->getPriority();
if(count($awaitPromises) > 0 && $currentPriority !== null && $currentPriority !== $priority){
//wait for concurrent promises from previous priority to complete
break;
}

$currentPriority = $priority;
if($handler->canBeCalledConcurrently()){
unset($handlers[$k]);
$promise = $handler->callAsync($this);
if($promise !== null){
$awaitPromises[] = $promise;
}
}else{
if(count($awaitPromises) > 0){
//wait for concurrent promises to complete
break;
SOF3 marked this conversation as resolved.
Show resolved Hide resolved
}

unset($handlers[$k]);
$promise = $handler->callAsync($this);
if($promise !== null){
$promise->onCompletion(
onSuccess: fn() => $this->processRemainingHandlers($handlers, $resolve, $reject),
onFailure: $reject
);
return;
}
}
}

if(count($awaitPromises) > 0){
Promise::all($awaitPromises)->onCompletion(
onSuccess: fn() => $this->processRemainingHandlers($handlers, $resolve, $reject),
onFailure: $reject
);
}else{
$resolve();
}
}
}
58 changes: 58 additions & 0 deletions src/event/AsyncHandlerListManager.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

/*
*
* ____ _ _ __ __ _ __ __ ____
* | _ \ ___ ___| | _____| |_| \/ (_)_ __ ___ | \/ | _ \
* | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
* | __/ (_) | (__| < __/ |_| | | | | | | | __/_____| | | | __/
* |_| \___/ \___|_|\_\___|\__|_| |_|_|_| |_|\___| |_| |_|_|
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* @author PocketMine Team
* @link http://www.pocketmine.net/
*
*
*/

declare(strict_types=1);

namespace pocketmine\event;

use function uasort;

/**
* @phpstan-extends BaseHandlerListManager<AsyncEvent, AsyncRegisteredListener>
*/
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<int, AsyncRegisteredListener> $listeners
* @phpstan-return array<int, AsyncRegisteredListener>
*/
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(...));
}
}
60 changes: 60 additions & 0 deletions src/event/AsyncRegisteredListener.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php

/*
*
* ____ _ _ __ __ _ __ __ ____
* | _ \ ___ ___| | _____| |_| \/ (_)_ __ ___ | \/ | _ \
* | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
* | __/ (_) | (__| < __/ |_| | | | | | | | __/_____| | | | __/
* |_| \___/ \___|_|\_\___|\__|_| |_|_|_| |_|\___| |_| |_|_|
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* @author PocketMine Team
* @link http://www.pocketmine.net/
*
*
*/

declare(strict_types=1);

namespace pocketmine\event;

use pocketmine\plugin\Plugin;
use pocketmine\promise\Promise;
use pocketmine\timings\TimingsHandler;

class AsyncRegisteredListener extends BaseRegisteredListener{
public function __construct(
\Closure $handler,
int $priority,
Plugin $plugin,
bool $handleCancelled,
private bool $exclusiveCall,
TimingsHandler $timings
){
parent::__construct($handler, $priority, $plugin, $handleCancelled, $timings);
}

/**
* @phpstan-return Promise<null>|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;
}
}
Loading
Loading