Pocketmine-mp: Asynchronous events

[SOF3]

TL;DR

• Async event allows you to use async task, wait for user response or do other time-consuming actions in an event handler.
• Async event is opt-in. This is not a compatibility-destructive proposal.

Example

This example shows how a plugin can translate PlayerChatEvent messages with an online API:

// Phase II: changes to the Player class for firing PlayerChatEvent
function Player::handleChat(){
    $event = new PlayerChatEvent(...);
    // $server->getPluginManager()->callEent($event); // the traditional way
    // the new way is like this:
    $server->getPluginManager()->callAsyncEvent($event, function(PlayerChatEvent $event){
        if(!$event->isCancelled()){
            foreach($event->getRecipients() as $recipient){
                if(!$recipient->isOnline()) continue; // Resources may have been invalidated after the event call ends.
                $recipient->sendMessage($event->getMessage()); // yes, I know it has some sprintf() stuff, but this is just an example
            }
    });
}

class TranslatePlugin extends PluginBase implements Listener{
    public function onPlayerChat(PlayerChatEvent $event){
        $event->pause();
        $this->getServer()->getScheduler()->scheduleAsyncTask(new TranslateTask($event));
    }
}

class TranslateTask extends AsyncTask{
    private $message;

    public function __construct(PlayerChatEvent $event){
        $this->storeLocal($event);
        $this->message = $event->getMessage();
    }

    public function onRun(){
        $result = Utils::getURL("https://example.com/translate?text=" . urlencode($this->message));
        $this->setResult($result);
    }

    public function onCompletion(Server $server){
        $event = $this->fetchLocal($server);
        $result = $this->getResult();
        if($result === false) $event->setCancelled();
        else $event->setMessage($result);
        $event->continue();
    }
}

There are no changes to PlayerChatEvent.php

Full RFC

Warning: The following RFC has more than 1500 words. If you don't want to read it, just read the TL;DR above and the "Expected use cases" at the end.

Introduction

Introduction: Why async events?

It is difficult to write a plugin that fetches data asynchronously, because many parts of the PocketMine API expect you to return a value immediately.

Currently, plugins usually use either of these two solutions:

• Allow the event to execute, then undo the event after the plugin finds out the event should have been cancelled.
  
  
  
  • For example, the plugin starts an AsyncTask to check whether a player is banned in PlayerPreLoginEvent. After the AsyncTask ends, if the player should be banned, the player is kicked.
  
  
  • But this already allows the player to join, and if the AsyncTask is slow enough, the player could already have done some damage.
  
  
  • MONITOR-level handlers will also fail to know that the event has been cancelled.
  
  
• Or, disallow the event from executing (cancel it), then redo the event after the plugin finds out the event should not have been cancelled.
  
  
  
  • For example, the plugin starts an AsyncTask to translate the message in PlayerChatEvent. After the AsyncTask ends, the plugin manually sends the translated message in PlayerChatEvent instead of calling PlayerChatEvent->setMessage().
  
  
  • But this will not work properly with other event handlers. This is a problem that not even #1798 can resolve.
  
  
  • The plugin also has to know the code that will be executed if the event is not cancelled. Sometimes this is not possible (like PlayerPreLoginEvent), otherwise it will be easily incompatible if the code is updated.
  
  

Each of these solutions has serious drawbacks. This RFC seeks to provide a better solution on behalf of the events API itself.

Proposal Phase I: The API

This RFC allows event handlers to properly pause the whole event. The message in PlayerChatEvent will not be sent until you are ready; the player in PlayerPreLoginEvent will stay "Connecting to server..." until you get your data.

The mechanism comprises two parts: event calling and event handling.

Event calling

Traditional way of calling events:

$PluginManager->callEvent($Event);

To call async events:

$PluginManager->callAsyncEvent($Event, function($Event){});

where the second parameter is the handler when the event completes.

It is yet to be decided whether callAsyncEvent or callEventAsync would be better.

Any event can be called asynchronously. Whether an event can be handled asynchronously depends on the caller, not the event type.

Event handling

An event handler can request the event execution to be paused by calling Event->pause(), and continue it with Event->continue().

If Event->continue() is never called, the event would theoretically get stuck forever. To prevent this, PocketMine should be responsible for tracking the executing async events. By default, an event can only pause for 5 seconds; beyond that time limit, PocketMine will trigger an error message and the event will continue executing (go to next handler, or complete). There would not be effective measures to stop the event from modifying event data later (e.g. while executing another handler), but since an error message is shown, it should be clear enough that there is something wrong, and the problem of timeout rather than the problem of concurrent event modification should be tackled.

Event->pause() accepts an optional argument float $timeout = 5 specifying the timeout in seconds. The implementation shall ensure the timeout not to exceed 60. Event subclasses may also override this method to disallow asynchronous event handling or limit the timeout. However, PocketMine shall hard-limit this timeout to 60 seconds to prevent stupid abusive behaviour.

(It is yet to be decided whether this should be in seconds or in ticks -- using ticks seems more reliable)

It is also proposed that a second optional argument, ?callable $onInterrupt, be accepted, which can be triggered when the timeout is reached. If this is null, the default behaviour (display error message) will be triggered.

In cases where the event is called with the traditional callEvent rather than callAsyncEvent, Event->pause() shall throw an InvalidStateException upon called.

In conclusion: the whole flow

callAsyncEvent ---> first handler (---> nth handler)* ---> onComplete

For each of the --->, the flow of control between is _undefined_ -- callAsyncEvent may or may not immediately result in an onComplete call, or there may be a few ticks in between even if there are no handlers at all -- developers should not rely on any observed or predicted behaviour regarding this, as this is subject to change.

As it currently is, the Event object should be recyclable _after the event call has fully completed_ (after onComplete is called; not during onComplete is called). Therefore, the implementation in PocketMine is responsible for resetting an Event to its initial state (in terms of what PocketMine will change) after onComplete is called.

Proposal Phase II: Migration of (some) current events calls to callAsyncEvent

Current events triggered by PocketMine shall be gradually migrated from callEvent to callAsyncEvent. This may involve complex refactoring in major sections of PocketMine's source code, and can be carried out throughout a longer span.

Backward compatibility

This RFC aims to preserve as much backward compatibility as possible, as this change is not totally necessary.

Phase I only: For event handlers

Traditional event handlers for traditional event calls do are not affected.

Phase II: For events migrated to callAsyncEvent

Event handlers must no longer assume that the event is fired in the same tick the event is eventually executed. This may need to minor but unobservable bugs with some hacky plugins.

For event callers

Calling events in the traditional way should not result in any semantic changes. If other (newer) plugins want to handle events from a non-async call, it's their own fault.

For event subclasses

There is minimal backward incompatibility for event subclasses. There are only edge cases with which this proposal will clash, namely:

• An old plugin declares an Event subclass with a method called pause/continue
• Developer assuming an Event subclass to be serializable/thread-safe. This is because handler queue may be stored in the Event class in the implementation.

Performance assessment

This RFC involves changes to the event calling mechanism. Its impact on performance is yet to be assessed after #1792 has been finalized, as it depends on #1792 (see below).

Tentatively, almost no impact on callEvent performance is expected. Minimal difference in performance between a callEvent and a callAsyncEvent call is expected, because the handler queue can be merged into an iterable array held in the Event instance. However, regular checking of timeouts may consume CPU unnecessarily, and shall be configurable.

Proposed versions

This RFC targets the API version that introduces #1792, or versions after that. Implementation will be based on #1792, because there may be semantic complexities regarding how a listener is triggered.

Expected ues cases

Regarding handlers

As stated in the introduction, this is mainly used for handlers that use AsyncTask to fetch some data. Nevertheless, it is also possible to suspend an event to wait for user input. If the event caller doesn't like this, they should override the pause method to limit the event to an acceptable amount of timeout.

One possible abuse include "queuing". For example, it may be tempting for a developer to pause a PlayerPreLoginEvent to wait for the server to get less full. This is an incorrect use case, because it is totally uncertain when an online player would quit voluntarily. Instead, this suspends the server's resources for some time (although limited to be 1 minute by specification in this RFC), and weakens existent mechanisms like player limit.

On the other hand, it is correct to use this, for example, to wait for an immediate user input. For example, a developer could suspend a PlayerCommandPreprocessEvent of /kill to send a modal form, "Do you really want to suicide?" This is a form of feedback that the user is expected to reply to immediately, and setting the timeout to a few seconds is reasonable. The post-execution of PlayerCommandPreprocessEvent is pretty light as well. (As a matter of fact, the forms API as currently in #1476 is pretty similar to how AsyncTask looks like)

Plugins can always register one LOWEST (or other priorities) and one HIGHEST priority handler respectively, where they start the AsyncTask at LOWEST without pausing the event there, and then pause the event and wait for the result at the HIGHEST (or otherwise any other appropriate) priority. This allows multiple plugins to execute their async tasks simultaneously. In this way, rather than adding up the waiting time for all events, only the waiting time of the slowest one will be the bottleneck. This concept is slightly complicated for elementary developers. On behalf of PocketMine API, it's possible although pretty difficult (by difficult, I mean we will spend weeks arguing which approach is better, just like... ykwim) to add a more convenient framework for such a mechanism. If this is to be done, it is out of scope of this RFC.

Regarding callers

Not all events should be allowed to be async. In particular, it is almost a "must not" for high-frequency events like PlayerMoveEvent to be async, because hundreds or thousands of other instances of the same event will execute during the async time.

[CtrlAltCuteness]

Suggestions:

1. Add another optional argument to go with the timeout argument (depending on value states if it is in ticks or seconds).

2. Make a new async creator to go with it, to allow an integeral and a floating-point number to specify the maximum (ticks & seconds), maybe with an enum like so:

3. warn / error / no message on first (can optionally specify which one first reached to throw an error);
4. if the two are set to any in (including a mix of) no message / warn on first reached, the only value possible for the enum should be to error on both being reached.

[SOF3]

@CtrlAltCuteness Allowing both ticks and seconds is unreasonable. I suggested using ticks because it may be correlated with how tasks execute on the server (which does not change with real-life time when the server is laggy. I just suggested real-life time too because that's what native timeout functions accept. But if both time systems are supported, it is an overkill. Timeout is, after all, just a mechanism to prevent bugs.
I don't understand what you mean in the second point, but note that the error operation can be overridden in a second parameter of Event->pause().

[falkirks]

I am inclined to agree with @SOF3 for the usage of ticks. You can always provide a lower bound in seconds given an amount of ticks (multiply by 20). However, when converting from seconds to ticks, you have an upper bound. I think in timeouts, a being given a little extra is better than a little less.

[Awzaw]

It is yet to be decided whether callAsyncEvent or callEventAsync would be better.

I much prefer callAsyncEvent and ticks. Nice proposal :-)

[CtrlAltCuteness]

Sorry @SOF3 as I think I was not fully awake at the time. The idea is to allow a unexpectedly slow local network (or relatively slow HTTP request & parse, etc.) to get data from a server database independent of the game speed, while knowing if it is too slow for a (example) game time-critical data fetch.

[SOF3]

Actually, the pause timeout is only to prevent people from forgetting to continue() the event. It is a real error, and it was not intended to help you handle expected timeout conditions.
In other words, the pause timeout should never occur in a production scenario. Developers are responsible for preventing this timeout before PocketMine notices it.
I doubt if I should even allow suppressing the error message.

[CtrlAltCuteness]

Understood. Thank you for clarifying, @SOF3

[SOF3]

Proposed addition.

Behavioural changes from the suggested result in the proposal:

• When an event handler calls pause(), instead of completely blocking the handler chain, it should continue executing other event handlers at the same priority and of the same HandlerList (i.e. they declare the same parameter type)
• The handler chain is only blocked when all handlers of the same priority in the same HandlerList have completed
• Event handlers can declare an additional @completePriority (which should be higher than or equal to @priority), then the handler chain will only be blocked when it reaches this additional priority, allowing more event handlers to be executed.
  
  
  
  • Edit: Event->pause() can carry an int $priority parameter (instead of @completePriority in the doccomment), where the new priority should be ≥ the handler's original @priority. Default equal to @priority.
  
  

[SOF3]

Obsoleted by #2470.