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

File watchers in DataContext are not closed consistently on process exit #22026

Closed
sync-by-unito bot opened this issue Jun 2, 2022 · 1 comment · Fixed by #22606
Closed

File watchers in DataContext are not closed consistently on process exit #22026

sync-by-unito bot opened this issue Jun 2, 2022 · 1 comment · Fixed by #22606
Assignees

Comments

@sync-by-unito
Copy link

sync-by-unito bot commented Jun 2, 2022

Current Behavior

Within the ProjectLifecycleManager, we do some cleanup when the process exits. Part of this cleanup includes closing all of our active file watchers (for specs, gitinfo, config files, etc.). As of chokidar 3.3.0, this close function is asynchronous, but the exit handler runs synchronously. This results in indeterministic results on close; sometimes we’re all good, sometimes we throw the SIGABRT. I assume this gets flakier as the number of active watchers increases. Removing the calls to close the watchers on exit altogether results in a consistent SIGABRT signal.

!Screen Shot 2022-05-25 at 11.41.15 AM.png|width=848,height=99!

This also appears to be a problem in our 9.x codebase. Example: https://github.com/cypress-io/cypress/blob/fe785749e80db32224186736be06b8b48ea8f271/packages/server/lib/watchers.js#L81-L82 I imagine we have similar indeterminism there by not awaiting these close calls. This would have been this way starting with our 8.5 release, which might explain the previously logged reports (https://github.com/cypress-io/cypress/issues/20101 )

I can prevent the SIGABRT by bumping chokidar back down to 3.2.3, when closing the watcher was synchronous. However, there are changes since then that we need to consume (M1 support, bug fixes), and a synchronous close was apparently not without its own problems. So we need to come up with a good way of getting in front of this exit event and make sure that these close calls resolve prior to exiting.

Desired Behavior

  • We should ensure all watchers are closed prior to exiting
  • Electron should not exit with a SIGABRT signal

Reproducible Steps

  1. (Note this this can be inconsistent, review attached video for reference)
  2. In the monorepo, launch cypress in open mode with DEBUG enabled (DEBUG=cypress:electron yarn cypress:open)
  3. Select any project, open e2e/ct tests in any browser, and start running a spec
  4. While the spec is running, close the Lauchpad (close button/Cmd+Q)
  5. Review console for evidence of electron closing with SIGABRT signal
  6. If no SIGABRT, repeat from 2.

Alternatively, remove the watcher.close() calls from https://github.com/cypress-io/cypress/blob/35ab63307ce7ce4058afd93f5195164d61091d4c/packages/data-context/src/data/ProjectConfigManager.ts#L486-L487 , open cypress, select a project, and close the Launchpad. You’ll see a SIGABRT in the electron logs consistently.

┆Issue is synchronized with this Jira Bug by Unito
┆Attachments: Screen Shot 2022-05-25 at 11.41.15 AM.png
┆author: Tyler Biethman
┆friendlyId: UNIFY-1816
┆priority: Medium
┆sprint: Fast Follows 1
┆taskType: Bug

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Jun 2, 2022

➤ Tyler Biethman commented:

I did some digging into how best we could do this during the investigation, prototype branch here: develop...tbiethman/UNIFY-1816-prototype ( develop...tbiethman/UNIFY-1816-prototype|smart-link )

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

Successfully merging a pull request may close this issue.

3 participants