Resolve #142 by adding a run() method #145
Conversation
| { | ||
| self::$level = 0; | ||
| }; | ||
| register_shutdown_function($f->bindTo(null, Loop::class)); |
There was a problem hiding this comment.
Hmm... this might not be the first shutdown function to be executed.
There was a problem hiding this comment.
I do not know of any way to prioritize that.
There was a problem hiding this comment.
If this was done in a file included immediately by composer this would be much more likely… but still not guaranteed.
Any particular reason for requiring the level to be 0 to call Loop::run()? We don't have this restriction for execute, even when passing the loop driver returned from Loop::get().
There was a problem hiding this comment.
Just not having run solves that, too.
There was a problem hiding this comment.
No, it doesn't. you cannot use setFactory() there event though you're at top-level...
There was a problem hiding this comment.
@bwoebi (1) I don't think we need setFactory() in shutdown hooks and introduce hacks for it. (2) I'd like to allow setFactory everywhere by removing the $driver = null reset here.
There was a problem hiding this comment.
I'm not sure the limited reward justifies the added complexity here, especially since the invocation order of shutdown callbacks can't even be relied upon.
There was a problem hiding this comment.
I pushed a commit dropping the level requirement… since shutdown function order can't be relied upon (and some apps even start the loop in a shutdown function) it just isn't worth trying to enforce it. Only so much we can do to protect users from themselves…
|
It seems like this doesn't actually solve anything. People can just use |
| @@ -430,3 +455,10 @@ private function __construct() | |||
| // intentionally left blank | |||
| } | |||
| } | |||
|
|
|||
| // Reset the $level in order to be able to use ::run() inside a shutdown handler | |||
| $f = function() | |||
There was a problem hiding this comment.
Isn't this polluting the variable scope?
There was a problem hiding this comment.
given the composer autoloader, no.
3b9246f to
5cae165
Compare
|
Can we close this one? |
As per: #142 (comment)
Also adding a shutdown function at the end in order to ensure that the loop level will be correct (i.e. people able to use Loop::run() in a shutdown function).
\cc @davidwdan @trowski @mbonneau