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

WebSocket Intermittent Infinite Loop #10831

Closed
simplerobot opened this issue Mar 24, 2023 · 1 comment
Closed

WebSocket Intermittent Infinite Loop #10831

simplerobot opened this issue Mar 24, 2023 · 1 comment
Assignees

Comments

@simplerobot
Copy link

I did this

I wrote code that created a WebSocket connection and used it do download data for multiple days.

I expected the following

I expected the WebSocket connection to be stable for extended periods of time. (Yes, I do know this is experimental.)

What actually happened

The connection remained stable for a while, but then started ignoring data and using 100% cpu. When this happened, curl_ws_recv was returning CURLE_OK, the recv parameter was set to 0, and the curl_ws_frame contained bytesleft of 0. My code repeatedly processed these phantom frames and called curl_ws_recv again for the next one.

The verbose output leading up to the problem was:

  • WS: received 160 bytes payload (0 left, buflen was 332)
  • WS: 168 bytes left to decode
  • WS:235 received FIN bit 1
  • WS: received OPCODE TEXT
  • WS: received 163 bytes payload (0 left, buflen was 168)
  • WS: 1 bytes left to decode
  • WS: plen == 1, EAGAIN

Following this, the last two lines were repeated indefinitely.

I dug into this behavior and believe I have found the source of the problem in ws.c. When curl_ws_recv (https://github.com/curl/curl/blob/master/lib/ws.c#L409) is called at the end of a frame (wsp->frame.bytesleft == 0) with one byte in the buffer (wsp->stillblen == 1):

  • curl_easy_recv on line 431 is not called due to the condition on line 428.
  • ws_decode on line 449 is called and returns CURLE_AGAIN since the buffer does not include enough bytes for a frame.
  • The break on line 453 results in return value of CURLE_OK with a buffer of length zero.
  • This repeats for each curl_ws_recv call.

I fixed this locally by updating:
https://github.com/curl/curl/blob/master/lib/ws.c#L453
< break;
> continue;
https://github.com/curl/curl/blob/master/lib/ws.c#L428
< if(!wsp->stillblen) {
> if(!wsp->stillblen || result == CURLE_AGAIN) {

So far, this has seemed to work, but I'm sure you can come up with something better.

curl/libcurl version

curl 7.88.1 (x86_64-pc-linux-gnu) libcurl/7.88.1 OpenSSL/3.0.2 zlib/1.2.11
Release-Date: 2023-02-20
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp ws wss
Features: alt-svc AsynchDNS HSTS HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL threadsafe TLS-SRP UnixSockets

operating system

Linux virt-linux 5.19.0-35-generic #36~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Feb 17 15:17:25 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

@bagder bagder self-assigned this Mar 24, 2023
bagder added a commit that referenced this issue Mar 28, 2023
Reported-by: simplerobot on github
Fixes #10831
@bagder
Copy link
Member

bagder commented Mar 28, 2023

thanks @simplerobot, your remarks make perfect sense but I think your change will lose a byte? When the loop happens due to result == CURLE_AGAIN it shouldn't store contents at index 0 in the data->state.buffer since in your example case there is already one byte there.

Let me show you my take at a slightly adjusted fix: #10856 - what do you think?

@bagder bagder closed this as completed in b19cbeb Mar 29, 2023
bch pushed a commit to bch/curl that referenced this issue Jul 19, 2023
Reported-by: simplerobot on github
Fixes curl#10831
Closes curl#10856
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants