When serializing & later unserializing exceptions thrown by clashing function calls

[brentkelly]

When serializing & later unserializing exceptions thrown by clashing function calls

[WyriHaximus]

Hey @brentkelly I'm a bit puzzled by your issue and your PR. Mainly because there is no technical reason to do any of this, but let me go through all of them and address them all, because I think we can solve you issue in a different way.

First off don't pollute the global namespace, and I know you can't help if because the framework of your choice did. Mainly telling you this that if you ever think of doing it you hopefully don't. Because it will another maintainers job harder.

Secondly, technically this is a bug in opis/closure and not in react/promise. So if you fix it there, it will not only fix the issue you have here, but also other (potential) buts for anyone using opis/closure.

But, thirdly, why are you serialising a Deferred instance? Doing so suggests you're transferring it to another thread or process, which means that any reference you use in the then/done calls will be lost as they don't exist in the new thread/process. But if you don't reference anything why not create the Deferred in the thread or process you're using it?

[gitneko]

Can you explain why you're serializing a Deferred instance? Doing so makes no sense, as the promise, resolver and rejector are not transferable. All information will be lost due to serialization.

If you are trying to do so to process a job in the background (as transferring to another thread or process), you need to solve this problem differently. You need to create your own Deferred in the background and use message passing between the two to resolve the Deferred in your "foreground task".

[WyriHaximus]

@gitneko Knowing Laravel a little bit, they absolutely love singletons for easy access. If the callables in then/done only talk to those singletons they will work fine and as intended. (Aside from the issue in opis/closure.) But we indeed need to know more to fully assess what is going on :).

[brentkelly]

Hi @WyriHaximus thanks for your reply.

I'll reverse your questions two & three so I can address them in that order:

why are you serialising a Deferred instance? Doing so suggests you're transferring it to another thread or process, which means that any reference you use in the then/done calls will be lost as they don't exist in the new thread/process. But if you don't reference anything why not create the Deferred in the thread or process you're using it?

Partially correct. Yes I am transferring it to another thread or process. But no, the reference to then/done calls is not lost. This is partially the point of Opis/Closure - allowing you to serialize closures (and releated variables / instances).

Unserializng works fine too except - aside from the parts outlined above.

Whatever the case - referencing the full namespaced function fixes the issue and it works perfectly. From there.

This allows us to easily use a clean promise-based approach to define code & state that should have its execution deferred to a future thread. Why would you want to do this? There are a ton of use cases that come to mind. But a couple off the top of my head:

• Fault tolerant scalability through queueing. A master thread can easily queue up a whole bunch of other threads to be executed by workers which pick up where the master thread left off.

• Headless API where you have some kind of state tracking that allows "if/then" type scenarios. E.g. API call 1 defines code that should execute if API call 2 happens.

My situation is the latter, working with a chatbot framework to use API calls.

All of this can be achieved directly with closures no problem already. So why do I care - well for all for the reasons that promises are superior to straight closures in the first place.

Secondly, technically this is a bug in opis/closure and not in react/promise. So if you fix it there, it will not only fix the issue you have here, but also other (potential) buts for anyone using opis/closure.

Agree - it is technically a bug opis/closure. I haven't deep dived into their code internals to see why or even if it is possible to fix but will log it with them. All I am suggesting is a minor tweak to your code base resolves the issue and means your promises can be serialized & unserialized - whereas at the moment they cannot be.

[brentkelly]

Here is a (very) basic example to illustrate it in action:

Thread 1:

use Opis\Closure\SerializableClosure;
use React\Promise\Deferred;

// write serialized content to this file so we can execute in two parts /
// different threads
$filename = __DIR__ . '/serialized';

$outside = [1, 2, 3];

$deferred = new Deferred();
$deferred->promise()
    ->then(function ($value) use ($outside) {
        echo "Received value to then is $value\n";
        echo "Outside is" . print_r($outside, 1) . "\n";
        return 'bar';
    })
    ->done(function ($value) {
        echo "Received value to done is $value\n";
    });

// write to a file for later use
$serialized = \Opis\Closure\serialize($deferred);
file_put_contents($filename, $serialized);

Thread 2:

use Opis\Closure\SerializableClosure;
use React\Promise\Deferred;

$filename = __DIR__ . '/serialized';
$serialized = file_get_contents($filename);
Opis\Closure\unserialize($serialized)
	->resolve('foo');

This results in:

PHP Fatal error:  Uncaught Error: Call to undefined function resolve() in closure://static function ($value = null) use (&$target) {
                        if ($target !== null) {
                            $target->settle(resolve($value));
                            $target = null;
                        }
                    }:4
Stack trace:
#0 /home/brent/code/php/vendor/react/promise/src/Deferred.php(36): React\Promise\Promise::{closure}()
#1 closure://function () use ($deferred) {
    $deferred->resolve('foo');
}(3): React\Promise\Deferred->resolve()
#2 /home/brent/code/php/index.php(40): {closure}()
#3 {main}
  thrown in closure://static function ($value = null) use (&$target) {
                        if ($target !== null) {
                            $target->settle(resolve($value));
                            $target = null;
                        }
                    } on line 4

Applying my pull request results in:

Received value to then is foo
Outside isArray
(
    [0] => 1
    [1] => 2
    [2] => 3
)

Received value to done is bar
Done

[brentkelly]

@gitneko Knowing Laravel a little bit, they absolutely love singletons for easy access. If the callables in then/done only talk to those singletons they will work fine and as intended. (Aside from the issue in opis/closure.) But we indeed need to know more to fully assess what is going on :).

Not using singletons.

opis/closure can handle resovling used objects, variables etc. Even detecting if the $this context is relevant & serializing that: https://docs.opis.io/closure/3.x/features.html#resolve-scope-this

They do however also say they can resolve function and constant names so I'll log an issue there too & see where that leads.

[brentkelly]

If you are trying to do so to process a job in the background (as transferring to another thread or process), you need to solve this problem differently. You need to create your own Deferred in the background and use message passing between the two to resolve the Deferred in your "foreground task".

I'm sure there are other ways to also tackle the problem. However I'm trying to implement a clean easy-to-use and read declarative interface to defining both code that will execute now, and code that will execute based on future state / events.

A promise is perfectly suited to describing this - given that is exactly what a promise does. In PHP the natural assumption is that a promise has to be declared and resolve in the same thread - for the reasons you outline above. But I'm exploring if Opis/Closure can get around a lot of these limitations. If it does so effectively (and it does seem to) it opens up a lot of cool use cases - namely that I can now declare a promise at one point in time, and then combined with some kind of storage I can resolve that promise at any point in the future.

I'm sure they may be some limitations & potential side effects - for a start if you're not careful you could easily have to start serializing & storing massive related object instances / datasets that were never intended to be written to storage. But at this stage it appears to work perfectly for my use case and provides a very clean & tidy interface for my team to use.

[brentkelly]

FYI here is a more complex proof of concept showing use of $this inside a serialized promise, which is then unserialized in a future thread. This works perfectly, resolving the promise & echoing protected properties of $this on resolve - proving that not only the resolve closure restored fine, but also the referenced $this instance.

https://github.com/brentkelly/serialized-promise-demo

[WyriHaximus]

Secondly, technically this is a bug in opis/closure and not in react/promise. So if you fix it there, it will not only fix the issue you have here, but also other (potential) buts for anyone using opis/closure.

Agree - it is technically a bug opis/closure.

And that is why we expect it to be solved there and not here. Not because we don't want to help you out, but because we have been spending a lot of time to improve our code quality for the upcoming v3 release (the master branch). And this would be a regression for that, to solve a problem that doesn't exist within the repository. Now looking at the example you posted later on this issue I'm pretty sure we can make what you are looking for work.

So to take you example, it looks like adding react/promise is only adding unnecessary bloat:

Thread 1:

use Opis\Closure\SerializableClosure;

// write serialized content to this file so we can execute in two parts /
// different threads
$filename = __DIR__ . '/serialized';

$outside = [1, 2, 3];

// write to a file for later use
$serialized = \Opis\Closure\serialize(function ($value) use ($outside) {
        echo "Received value to then is $value\n";
        echo "Outside is" . print_r($outside, 1) . "\n";
        return 'bar';
    });
file_put_contents($filename, $serialized);

Thread 2:

use Opis\Closure\SerializableClosure;
use React\Promise\Deferred;

$filename = __DIR__ . '/serialized';
$serialized = file_get_contents($filename);
echo "Received value to done is ", (Opis\Closure\unserialize($serialized))('foo'), "\n";

[gitneko]

A promise is perfectly suited to describing this - given that is exactly what a promise does. In PHP the natural assumption is that a promise has to be declared and resolve in the same thread - for the reasons you outline above. But I'm exploring if Opis/Closure can get around a lot of these limitations. If it does so effectively (and it does seem to) it opens up a lot of cool use cases - namely that I can now declare a promise at one point in time, and then combined with some kind of storage I can resolve that promise at any point in the future.

Not even opis can get around this limitation. Serializing $this may work, but unserializing it will lead to a complete new unrelated instance with no references to the previous instance. You can see this by just serializing and unserializing any object in the same thread.

There's no synchronization, no notification, nothing that makes both $this instances the same and behave the same. That means, yes, you get the same behaviour in the same thread you're resolving the promise, the callbacks get called (but only in the same thread) etc., but the other thread does not know about that, your promise does not resolve at all, the callbacks don't get called.

If you need that, you need as I've described before a message passing approach and keep two distinct deferred objects on each side and create these reactive when a "job arrives". I've gone down that road before.
If you don't need that, you can throw the deferred instance on the serialize side away, since it's just garbage you're creating.

Since it's a bug in opis/closure, the bug should be fixed in opis/closure and not worked around that in a library.

[brentkelly]

So to take you example, it looks like adding react/promise is only adding unnecessary bloat:

Yes the example is not attempting to show a use case for where promises are needed. As above the example was designed to be as simple as possible for the sole purpose of demonstrating the problem.

[brentkelly]

Not even opis can get around this limitation.

It can, and does. See below.

Serializing $this may work, but unserializing it will lead to a complete new unrelated instance with no references to the previous instance. You can see this by just serializing and unserializing any object in the same thread.

Yes I understand how serializing works.

There's no synchronization, no notification, nothing that makes both $this instances the same and behave the same.

Agree they are not references to the same instance (as in memory location), but they are duplicates (as much as serialization can achieve) - in this manner they are essentially the same & they do behave the same - a few exceptions aside (discussed below).

That means, yes, you get the same behaviour in the same thread you're resolving the promise, the callbacks get called (but only in the same thread) etc., but the other thread does not know about that, your promise does not resolve at all, the callbacks don't get called.

Actually you're wrong here.

opis/closure successfully serializes the entire promise, all of its callbacks, and related objects.

Yes when they unserialize they are essentially clones of the original instances. And yes they won't be synchronised to anything that occured after they were serialized - which is kind of the point given you don't want them destroyed along with the original instance when the thread terminates.

Also obviously if referenced objects contain active connections (e.g. database connections) that usually get broken during serialization then those classes will need to declare __wakeup methods or something similar to support the process. This is just serialization in general though - nothing special regarding promises or closures.

Look at my example above: https://github.com/brentkelly/serialized-promise-demo. This is a working demonstration of it doing precisely what you say can't happen:

1. index.php welcomes a Guest, and then creates a promise (inside Guest) with future execution farewelling the guest by the provide name (stored as a property of Guest)
2. For the promise to correctly resolve & farewell the guest it has to:
   a) serialize and unserialize (reconstruct) the original promise correctly, including all of its callbacks
   b) restore the $this instance representing the Guest, with its properties (name) correctly restored.
3. The promise is serialised and written to a file.
4. Thread 1 terminates - giving you a command to execute to resolve the promise in a new thread.
5. Now in a separate thread, you execute the provided command (resolve.php), and there in a totally independant thread it resolves the promise correctly, and farewells the Guest by name - demonstrating not only did the promise unserialize, restoring all callbacks, but so did the instance the then closure was generated within.

[brentkelly]

Appreciate your time & thoughts here guys. Thanks.

I have an implementation of this working perfectly now aside from the issue my PR addresses. Obviously there are the usual side effects of serializing a programmer needs to be aware of (as illustrated in the post above) but aside from that it seems to be working just as well as any serialized closure solution.

While the bug exists in opis/closure, they are obviously doing some pretty complicated stuff flirting with the boundaries of what serialization can & can't do in PHP. Hopefully it will be possible to fix - I will see how they respond to my issue.

While I appreciate wanting to keep a code base clean with a future version 3, I was hoping you would consider a 2.8.1 or something given it is a very minor change, arguably makes the code easier to read, and has no risk of introducing any issues. No disrespect (this project is great) but as you seem to imply with your attempts at version 3, it's not like 2.8.0 is the gold standard of best practice clean code so such a small change would not IMO detract at all from the current code base.

Obviously it's your opinion that counts here though & it sounds like that decision has been made - so I'll close this & have to run on a fork or switch to a more compatible project unless opis/closure can resolve things.

Thanks.