Any way to "reset" the loop?

[bpolaszek]

Hey there! 👋

I stumbled upon a kinda weird bug when using the loop in my CI, where some tests sometimes perform Loop::stop() when some others launch Loop::run() (though they don't run concurrently). This leads to fatal errors as raised here, although they would not be raised when running tests one by one. 🤨

I absolutely don't know how this is handled in the background as the fiber thing is kinda too low level to me, but it looks like the loop is sometimes left in some sort of stale state (which is not supposed to happen in production, as the loop presumably runs once). 🧐

Spawning a new loop before each test is easy to do and turns the tests to green, except it relies on deprecated code. Do you know a better way to handle this? Otherwise would it be relevant to add a Loop::reset() method? 💡

As a side-feature-request, a Loop::isRunning() method would be a great help to avoid this kind of tricks (yeah, this method can be invoked from an X-Framework app as well as from a regular PHP app, meaning the loop, in this case, just has to run to execute the promise and stop immediately afterwards) because we cannot know, from the outside, whether the loop is actually running or not.

Thanks! 🙏
Ben

[SimonFrings]

Hey @bpolaszek, thanks for bringing this up 👍

I ran into something similar a while ago in reactphp/socket#283. I would recommend to close everything at the end of your tests (Loop/connections etc.) to avoid leftovers.

I also noticed that you're using an older version of reactphp/async (futureTick got removed in reactphp/async#32) and reactphp/eventloop (you're using the Facotry::create() instead of Loop::get(), refs #226, #229 and #231). Maybe updating to the current versions will do the trick but I don't know for sure.

I hope this helps!

[bpolaszek]

Hi @SimonFrings,

Thank you for taking some time to look at this.

Regarding react/async, I just upgraded to the latest commit. Now I get a different error, but that's not the point.

How do you close the loop at the end of tests? By calling Loop::close()?
So, that's what I just did, but a test is still failing when the whole test suite is ran. It doesn't fail when ran alone.

The only way I found to have the test suite running green is to uncomment this line. And I'm using the deprecated Factory::create() just because Loop::get() doesn't work, as the latter returns an (already used) singleton instead of a fresh instance.

So, as my problem is to rely on the deprecated Factory::create() method, how am I supposed to handle this?

[clue]

@bpolaszek I agree that this can be cumbersome sometimes, but I concur with everything @SimonFrings said: As a rule of thumb, you should never call Loop::stop() unless you're absolutely sure there's no alternative.

I understand it's tempting to call Loop::stop() in tests to interrupt the execution, but what it really means is that there are still listeners attached to the loop that will resume running the next time you call Loop::run(). As an alternative, I would highly recommend cleaning up any listeners as part of each test. Cleaning up any listeners allows the loop to stop automatically when it has nothing left to do (no timers and streams attached).

You can take a look at the referenced tickets and also #237 for some additional discussion about this. I'm happy to discuss if you feel there's anything else that needs to be done on our end here.

I hope this helps! 👍

[bpolaszek]

I didn't remember participating in #237 so it looks like that same issue happened to me earlier and I ran into it again! 😅

Well, then I'll first rely on the deprecated Factory::create until its sunset, as there's no such thing as a "listener collector" (which is, from my POV, the Loop instance's job) we could invoke to clear them all.

Cheers!