Hey Josh,

> Apologies, this is a long one!
>
> This RFC strikes me as being very dangerous. Implicitly allowing code
> which is synchronous by design to be executed asynchronously seems
> sure to lead to very subtle, unpredictable, confusing and dangerous
> bugs.
>

Thank you for sharing your thoughts, I'll respond inline for better
context, but shortened some paragraphs for better readability.


> [...]
>
> For example, the PHP ecosystem is full of code like this:
>
>     private function capturePayment()
>     {
>         $paymentRequest = preparePaymentRequest($this->currentOrder);
>         $this->paymentGateway->capturePayment($paymentRequest);
>
> $this->currentOrder->setTransactionId($paymentRequest->getTransactionId());
>     }
>
> In the above code snippet, the developer has relied on the fact that
> the mutable value $this->currentOrder will be the same before and
> after the call to $this->capturePayment() [...]
>

This is a very valid concern to have. However, this code won't simply break
if executed asynchronously.
It only breaks if the same method (or other methods making use of the same
state) is executed concurrently on that object.


> [...]
>
> - These race conditions will cause problems so rarely that developers
> will not even realise that their programs contain dangerous bugs. [...]
> - Changes very, very far away from your code can cause your code to
> become unexpectedly asynchronous. [...]
> - As a library author, there is no way of knowing how your code is
> going to be used, and what injected dependencies might use fibers
> internally.
>
> The counter arguments:
> - "The above code snippet is suboptimal, it shouldn't assume that
> mutable state remains unchanged following some IO, and the order
> should be passed in as an arg." [...]
> - "Fibers should only be enabled in async first programs." and "We
> could flag that certain libraries are fiber compatible or
> incompatible." [...]
>
> Consider the following:
>
>     private async function capturePayment()
>     {
>         $paymentRequest = preparePaymentRequest($this->currentOrder);
>         await $this->paymentGateway->capturePayment($paymentRequest);
>
> $this->currentOrder->setTransactionId($paymentRequest->getTransactionId());
>     }
>
> Looking at the above code, I can see the race condition. It becomes
> explicit. So I change the code and communicate explicitly via the type
> system that this function is asynchronous and that my code expects
> PaymentGateway::capturePayment() to be asynchronous.
>

If you can see the race condition here, you can probably also see the race
condition in the original snippet above.

The overall issue you're describing is thread safety and mutable state. It
exists with fibers, but a very similar issue exists with explicit async /
await.
Just because something supports asynchronous I/O, doesn't mean it also
supports concurrent operations on its state.

It might be a good idea to introduce some guarding concept in a follow-up
RFC, so objects can easily protect against being used concurrently e.g.

```
private synchronized function capturePayment()
{
     $paymentRequest = preparePaymentRequest($this->currentOrder);
     $this->paymentGateway->capturePayment($paymentRequest);

 $this->currentOrder->setTransactionId($paymentRequest->getTransactionId());
 }
```

"synchonized" isn't the right keyword here, but I can't think of a better
name right now.


> Regarding PHP's type system not supporting generics, and the
> associated advantages of this RFC over promises; I agree that this is
> a problem with promises. But the lack of generics in PHP already means
> we have no language-level type safety on data structures, which is
> certainly worse than not having type safety on promises. Psalm et al
> have provided reasonable solutions to this issue.
>

Being able to use the type system is a concern the RFC solves, but that's
rather just nice-to-have. The relevant problem is the call stack pollution
or "What color is your function" problem. It simply doesn't make sense for
applications and libraries to offer an async and non-async interface for
everything. Due to the "What color is your function" problem, there can't
be a gradual migration to non-blocking I/O, which will mostly result in
non-blocking I/O simply not being supported by the majority of libraries,
making it basically non-viable in PHP.

Best,
Niklas

Reply via email to