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

hot reload intermittently fails #6

Open
eyeree opened this issue Jan 8, 2022 · 1 comment
Open

hot reload intermittently fails #6

eyeree opened this issue Jan 8, 2022 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@eyeree
Copy link

eyeree commented Jan 8, 2022

Occasionally evt.Data is 'reload\nreload' instead of just 'reload', causing the console.error branch to be taken in the reload script. I'll see 3-4 successful reloads, then 1-2 failed reloads, then a few successful, etc.

I'm using the linux-x64 server running on Unbuntu under Windows 10 using WSL 2.0. I see the behavior in Chrome, Firefox, and Edge running under Windows on the same host.

It's hard to tell, but it might be happening more often if I have Chrome, Firefox, and Edge all open at the same time, vs. just one of them. When I do have more than one browser open, they ALMOST always either succeed or fail together, but occasional one will work while the others fail.

Seems like race condition of some sort. Wouldn't be noticeable if the reload script ignored the value.

@Falldot Falldot added the bug Something isn't working label Feb 10, 2022
@Falldot Falldot self-assigned this Feb 10, 2022
@jaens
Copy link

jaens commented Mar 18, 2022

This is caused by the following code, which for some mysterious reason racily1 reads all still pending messages from the websocket's broadcast channel and separates them with \n:

n := len(c.send)
for i := 0; i < n; i++ {
w.Write([]byte{'\n'})
w.Write(<-c.send)
}

In this case, what's happening is that there are two consecutive reload messages in the channel which then cause this corruption.

WebSocket messages are supposed to be atomic units, so not sure why that code even exists - and in any case, it's impossible to implement message merging correctly (ie. non-racily) with that code (or basically, with any implementation that uses a single raw byte[] channel - for multi-part messages, the channel needs to either be framed with message lengths or use a terminator).

Footnotes

  1. Note that len(c.send) can further increase due to new messages while in the for loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants