Static Class Prototype

[AndrewCarterUK]

For those not in the know, this follows on from the discussion in #14 and attempts to solve the problem of being able to access the event loop without injecting it as a dependency anywhere.

The solution that I favour discussed in #14 is represented in this PR. I believe it represents a happy medium between injecting the event loop everywhere and global god classes. The life time of an event loop is easily defined by the scope within which it is invoked.

Example usage:

use Interop\Async\EventLoop;
use AsyncProject\LibEvDriver;

// ...

EventLoop::execute(function () {
    // This callback will be run within the scope of the specified event loop driver

    // ...

    EventLoop::onReadable($inputStream, $inputStreamHandler);
    EventLoop::onWritable($outputStream, $outputStreamHandler);
}, new LibEvDriver());

[AndrewCarterUK]

There is also an argument that this should live in a separate repository. Unlike the interface for the event loop driver (which we cannot change after publishing) - this is a code file which needs unit tests and could require maintenance. Also, if you are implementing the event loop driver you likely don't need to use this class - as it's intended for consumers.

[joelwurtz]

missing static ?

[AndrewCarterUK]

Thank you, there were a few of those!

[kelunik]

I think we can keep both in one repository, I don't really care. Any application requires both anyway. Just loop drivers might not need it, but even they might want to use the global accessor in unit tests.

[sagebind]

Even further, a number of packages that provide loop drivers also provide a lot more core functionality as well. Just look at the Icicle and Amp core packages. It makes sense I think to keep them together.

[AndrewCarterUK]

@WyriHaximus @trowski @assertchris

Can I have your thoughts on this approach? It's quite a significant deviation from our initial ideas and has some significant implications (requiring a test suite [phpunit?], static state, etc...). As a result, I'd like as many thoughts on as possible before investing significant time in this.

If anyone has any alternative suggestions then a separate pull request (such as @CoderStephen 's) would make it easy to compare all of our options more objectively.

[trowski]

I have a few reservations about this implementation:

• inScope() should be part of get(), then always use get() to access the loop instance.
• I would still like to see set() and drop() methods in addition to execute()/with()
• Methods should have return type declarations (though we may want to settle the watcher ID vs. object issue)
• Perhaps execute() should throw if there is an event loop set that is running? Running an event loop within another loop seems like a bad idea.
• Should there be a method to check if the loop is running? isRunning()? Maybe another to check if a loop instance is set?

[bwoebi]

• Whether inscope() is part of get() or not doesn't really matter...?
• What would you need set() and drop() for?
• Return type decls are nice, but are we targeting 7.0+ only? (see also Minimum PHP Version #2)
• The point is pretty much to allow nesting of event loops. To block everything else from running, but still dispatch a separate loop. (e.g. in a register_shutdown_function where state may be inconsistent, but we still can run a new event loop on top of the current one to guarantee some task to be run)
• No, we don't need such a method, we can suppose to always be inside an event loop (everything else would error then). If you aren't in an event loop and accessing it (apart form execute()) that's a programmer mistake.

[trowski]

inScope() is a "side-effect" style function that just bothers me. Technically it doesn't matter, but I think return self::get()->method(...) is more elegant than calling a method to check if a variable is not null before using it.

I assumed we were targeting 7.0+ as some methods use scalar types for parameters.

I was thinking of using set() and drop()for testing, but since execute() automatically "drops" the loop instance after running, maybe the methods aren't needed.

Generally I was considering a nested event loop to be an error, but if this is something we want to allow, then this works.

[assertchris]

PHP 7+

[assertchris]

I don't mind this approach, but I'm starting to think that a separate repo would be better. If we make a breaking change to the impl, the interfaces get a major version bump? :(

[sagebind]

If you still are considering the set()/drop() approach as an alternative, I updated #13 so that we can compare and discuss.

[sagebind]

Here are my thoughts on the discussion:

• I like the idea of an isRunning() method. 'nuff said for that.
• The execute() closure-style isn't particularly bad, but the design seems optimized for the use-case of multiple event loops in one program, which I thought isn't something we were trying to encourage (not that it should necessarily be forbidden).
• Using a closure makes scope clear in the file in which execute() is called, but I don't think it helps much anywhere else in a program.
• There are technical issues with closures and multithreading; absolutely requiring a closure in order to set up the event loop initially may or may not be an issue. I don't have a solid answer to that. Similarly, how will this affect forking inside an event loop scope? There needs to be a way to simply discard all event loops from the parent process cleanly, which I'm not sure the strict execute() approach allows. Forking doesn't treat previous scoping nicely in that regard.

@assertchris I'm not against putting this in a separate repo, but any package using the interface is almost for sure going to use the static class too.

[WyriHaximus]

For the sake of have the discussions in one place we can keep it in this repo for now and move it later on when things matured more.

[kelunik]

Should stopping the driver unset it?

[bwoebi]

Agree; else you have no possibility to intentionally run destructors etc.
Especially important for tests.

[AndrewCarterUK]

Stopping the event loop will cause the run() method to return, which will trigger that behaviour inside execute()?

[kelunik]

You're right.

[kelunik]

@AndrewCarterUK Typo in your commit message maknig. Otherwise I'm fine with merging it for now, so I can base my work for #16 on it.

[AndrewCarterUK]

Merged so you can continue with the PR discussed in #16