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

fix(mxDebug): prevent spinning if connection closed #19

Closed
wants to merge 1 commit into from

Conversation

michaelfig
Copy link
Collaborator

@michaelfig michaelfig commented Feb 7, 2022

See #18 (comment) for steps-to-reproduce.

If the other side of the xsbug connection closes, xsnap and xsnap-worker would go spinning up 100% CPU.

If recv or read returns a count of 0, we should close our end of the connection, too.

@michaelfig michaelfig self-assigned this Feb 7, 2022
@phoddie
Copy link
Collaborator

phoddie commented Feb 7, 2022

This appears to be the functional equivalent of the macOS implementation of fxReceive expressed slightly differently. For consistency / maintainability, you might want to consider using the same code here.

@michaelfig
Copy link
Collaborator Author

This appears to be the functional equivalent of the macOS implementation of fxReceive expressed slightly differently. For consistency / maintainability, you might want to consider using the same code here.

It's different. Look specifically at what happens when read or recv returns 0.

@phoddie
Copy link
Collaborator

phoddie commented Feb 7, 2022

If it is different, it will take more time to look into this. the macOS version has been used as-is for years (and years) and doesn't exhibit the behavior described.

@michaelfig
Copy link
Collaborator Author

If it is different, it will take more time to look into this. the macOS version has been used as-is for years (and years) and doesn't exhibit the behavior described.

Do note that the infinite loop is not within this function. It's some caller that isn't interpreting the connection closed (a debugBuffer of ['\0']) correctly, probably assuming that fxReceive would have closed the connection on its behalf.

@phoddie
Copy link
Collaborator

phoddie commented Feb 8, 2022

If the other side of the xsbug connection closes, xsnap and xsnap-worker would go spinning up 100% CPU.

I wasn't able to reproduce by simply quitting xsbug. When xsbug cleanly exits, it sends an abort command to the running app which shuts it down.

Through some experimentation, I was able to reproduce the problem with these steps:

  1. Launch xsbug
  2. Launch a project that connects to xsbug
  3. Pause execution of the project by breaking in xsbug (Debug menu > Break item)
  4. Force quit xsbug (not a clean exit)

That loops forever calling read which returns 0 as you observed.

On macOS, the problem was solved by changing these lines to:

		count = read(CFSocketGetNative(the->connection), the->debugBuffer, sizeof(the->debugBuffer));
		if (count <= 0) {

@michaelfig
Copy link
Collaborator Author

On macOS, the problem was solved by changing these lines to:

		count = read(CFSocketGetNative(the->connection), the->debugBuffer, sizeof(the->debugBuffer));
		if (count <= 0) {

Would you be able to make this change to the upstream XS? When that's done, I'm happy with closing this PR.

@phoddie
Copy link
Collaborator

phoddie commented Mar 28, 2022

Thank you for confirming that the proposed change resolves the issue for you. I've applied the change to the macOS, Windows, and Linux builds. It will be in the next update to the Moddable SDK repository, later this week.

@michaelfig michaelfig closed this Mar 28, 2022
mkellner pushed a commit to Moddable-OpenSource/moddable that referenced this pull request Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants