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

embedded:io/socket/listener locks up Win simulator #1428

Closed
cmidgley opened this issue Oct 21, 2024 · 3 comments
Closed

embedded:io/socket/listener locks up Win simulator #1428

cmidgley opened this issue Oct 21, 2024 · 3 comments

Comments

@cmidgley
Copy link
Contributor

Build environment: Windows
Moddable SDK version: 5.1.0
Target device: Win simulator.

Description
I'm trying to get embedded:network/http/server to work on win simulator, but it locks up (does not respond) the simulator process and requires it to be hard terminated. The examples for listener/httpserver and listener/listen exhibit the failure, indicating the bug is likely related to listener.

Both examples work on esp32, but for httpserver a trivial bug in examples/io/tcp/websocket/WebSocket.js on line 51 needs to be fixed to not throw an error (doesn't affect the crash issue). Changeif (href) to else if (href), as the prior check for object has identified this is an attachment and there is no href.

Steps to Reproduce

  1. Run examples/io/listener/listen or examples/io/listener/httpserver on windows simulator
@phoddie
Copy link
Collaborator

phoddie commented Oct 21, 2024

Last I checked, the unit tests for Listener passed on Windows. We'll take another look.

@phoddie
Copy link
Collaborator

phoddie commented Oct 22, 2024

The listener on Windows has a bug where it would end up as a blocking socket, rather than non-blocking. This led to the stalls you see. There are two places to fix that., Line 176:

  			ioctlsocket(tcp->skt, FIONBIO, &nonBlocking);

And line 577:

		ioctlsocket(listener->skt, FIONBIO, &nonBlocking);

There is also a smaller issue – a small memory leak if an exception occurs in the constructor – which will fixed when the above changes are integrated into the SDK.

@cmidgley
Copy link
Contributor Author

Patch applied and fix confirmed. Thank you!

mkellner pushed a commit that referenced this issue Nov 13, 2024
mkellner pushed a commit that referenced this issue Nov 13, 2024
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

No branches or pull requests

2 participants