Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interrupt test workers caught in an infinite loop #1378

Closed
novemberborn opened this issue May 9, 2017 · 4 comments
Closed

Interrupt test workers caught in an infinite loop #1378

novemberborn opened this issue May 9, 2017 · 4 comments
Labels

Comments

@novemberborn
Copy link
Member

As suggested by @molsson in #1377 (comment), AVA should "detect bugs where a synchronous function accidentally ends up in an infinite loop."

Users might expect --timeout covers this, but it merely calls fork.exit(), which only sends an init-exit message to the worker. If the worker is blocked then it'll never process that message, and it won't exit.

We need to decide whether to detect this case purely with --timeout, in which case we can kill the worker process if it doesn't exit within a few seconds (perhaps the --timeout period?).

Alternatively we could always detect this, by sending occasional pings to the worker.

@ORESoftware
Copy link

ORESoftware commented May 11, 2017

upon --timeout, send a SIGINT, followed by SIGKILL after a grace period...?

@novemberborn
Copy link
Member Author

Users might expect --timeout covers this, but it merely calls fork.exit(), which only sends an init-exit message to the worker. If the worker is blocked then it'll never process that message, and it won't exit.

With #1722 we'll force the worker to exit without relying on IPC. If we fix #583 then we can clearly communicate which tests were active and thus may have caused the infinite loop.

#1718 proposes we don't automatically exit the process once the last hook has completed. This makes processes that never drain their event loop indistinguishable from those that have an infinite loop, as far as the main process is concerned.

I suggest we set a default timeout to say 60 seconds, and then force any active worker processes to exit.

@sindresorhus
Copy link
Member

Alternatively we could always detect this, by sending occasional pings to the worker.

👍

@novemberborn
Copy link
Member Author

I believe our current timeout logic is sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants