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

Further workaround to try to push node.js event loop forward #1511

Merged
merged 1 commit into from
Sep 28, 2017

Conversation

scottnonnenberg
Copy link
Contributor

We're still having problems with the Node.js websocket in Electron, tracked here: #1075

This takes our workaround and amps it up. Instead of calling setImmediate after making a web request, we do it every second. I'm hoping that this extreme approach solves the problem, and then we can do the work to pare back as necessary.

@liliakai
Copy link
Contributor

This certainly seems extreme. I don't really understand how the node event loop is integrated in the browser window process well enough to understand how it could be blocked and why this strategy might help. Should this behavior be limited to linux?

Do we think this is related to process throttling that occurs when the window is in the background? If so, maybe we can try disabling that in webPreferences.

@scottnonnenberg
Copy link
Contributor Author

scottnonnenberg commented Sep 28, 2017

It should have very, very little overhead, so I don't think it needs to limited to Linux.

Now, as far as the why... my mental model is that sometimes things get stuck on the Node.js side. That simple repro of two quick web requests proves it, and also shows that a little push from us can help. Our first workaround's single setImmediate call after every web request helps, but with a long-lived network connection like a websocket, perhaps it needs more frequent pushes? Not only that, but a slower internet connection may take longer than one second to respond, and in that case our workaround fired too early.

@scottnonnenberg scottnonnenberg merged commit 2d650bd into master Sep 28, 2017
@scottnonnenberg scottnonnenberg deleted the linux-workaround branch September 28, 2017 21:47
scottnonnenberg added a commit that referenced this pull request Sep 29, 2017
Make long-lived socket connections more reliable (#1511)

Show offline state faster on loss of network access (#1512)

Notifications:
  - Only show notifications when a large backlog download is complete
    (#1507)
  - Ensure final message before 'empty' is ready for notification
    (#1522)

Ensure we always replace $name$ variable in strings (#1520)

Update strings for fa, no, pt_BR, pt_PT, ro, zh_CN, zh_TW (#1517)

Update electron to v1.6.14 to get security fix (#1519)

Eliminate warning on Windows installation with code-signing (#1513)

Dev:
  - Move logging to disk via bunyan - should make message processing
    faster! (#1506)
  - Only retry messages on startup, not every reconnect (#1510)
  - Log call messages instead of throwing error (#1504)
  - Redact group ids in logging
    (4c48d12)
  - Remove manifest.json from project
    (3a3f249)

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

Successfully merging this pull request may close these issues.

2 participants