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.

Sign up for GitHub

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

Jump to bottom

[Regression] libcurl does not finish CURLOPT_UPLOAD request after a connection lost without data tranfer #13260

Closed
ligurio opened this issue Apr 2, 2024 · 7 comments
Closed

[Regression] libcurl does not finish CURLOPT_UPLOAD request after a connection lost without data tranfer #13260

ligurio opened this issue Apr 2, 2024 · 7 comments
Assignees
@icing

Comments

@ligurio
Copy link
Contributor

ligurio commented Apr 2, 2024 •
edited

I did this

There was an issue in Tarantool 1 due to regression in libcurl (#11769), it was reported on 23 Aug, 2023. Issue was fixed in upstream on 4 Sept, 2023 and fix was confirmed by our tests for Tarantool. Unfortunately, this fix was ruined by refactoring made in commit d7b6ce6 ("lib: replace readwrite with write_resp") [3] on 1 Dec 2023, and now it is reproduced again.

I expected the following

curl/libcurl version

8.6.0+

operating system

uname -a: Linux pony 6.5.0-26-generic #26~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Tue Mar 12 10:22:43 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Ubuntu 22.04.4 LTS

Footnotes

  1. https://github.com/tarantool/tarantool/issues/9086 ↩ ↩2
@icing icing self-assigned this Apr 2, 2024
icing added a commit to icing/curl that referenced this issue Apr 3, 2024
@icing
paused upload on completed download, assess connection
0102644 
- a transfer with a completed download that is still uploading
  needs to check the connection state when it is PAUSEd, since
  connection close/errors would otherwise go unnoticed.
- refs curl#13260
@icing icing mentioned this issue Apr 3, 2024
Closed
@icing
Copy link
Contributor

icing commented Apr 3, 2024

I tried to reproduce the situation in a curl test case, so that we can avoid regressions on this in the future. The case where I could reproduce a stall with the server going aways is:

A transfer

  • where the upload is PAUSED
  • where the download is done
  • the server closes the connection

If this is indeed the situation that you are in with the etcd server (speculating here), then this points to a poor HTTP API design. Which you can probably do nothing about, I just wanted to mention it.

@icing
Copy link
Contributor

icing commented Apr 3, 2024

Could you verify if #13271 addresses your issue?

@ligurio
Copy link
Contributor Author

ligurio commented Apr 3, 2024

Could you verify if #13271 addresses your issue?

Sure, will do and report soon.

@ligurio
Copy link
Contributor Author

ligurio commented Apr 3, 2024

The issue is still present with a patch on top of the master (60971d6).

Curl's output:

Captured stderr:
* !!! WARNING !!!
* This is a debug build of libcurl, do not use in production.
* STATE: INIT => CONNECT handle 0x5d9a3121cbc8; line 1910
* Curl_client_reset(), clear readers
* Added connection 0. The cache now contains 1 members
* Host localhost:2379 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
* STATE: CONNECT => CONNECTING handle 0x5d9a3121cbc8; line 1964
*   Trying [::1]:2379...
* connect to ::1 port 2379 from ::1 port 52424 failed: Connection refused
*   Trying 127.0.0.1:2379...
* Connected to localhost (127.0.0.1) port 2379
* STATE: CONNECTING => PROTOCONNECT handle 0x5d9a3121cbc8; line 2072
* STATE: PROTOCONNECT => DO handle 0x5d9a3121cbc8; line 2101
* cr_exp100_read, start AWAITING_CONTINUE
> POST /v3/watch HTTP/1.1
Host: localhost:2379
Transfer-Encoding: chunked
Accept: */*
Connection: Keep-Alive
Keep-Alive: timeout=1
Expect: 100-continue

* STATE: DO => DID handle 0x5d9a3121cbc8; line 2197
* STATE: DID => PERFORMING handle 0x5d9a3121cbc8; line 2315
* cr_exp100_read, AWAITING_CONTINUE, not expired
* HTTP 1.1 or later with persistent connection
< HTTP/1.1 100 Continue
< 
* cr_exp100_read, pass through
* cr_in_read(len=65524, total=-1, read=0) -> 0, 0, 0
* nread <= 0, server closed connection, bailing

@icing
Copy link
Contributor

icing commented Apr 3, 2024

Thanks @ligurio for the log output. I believe I have not a correct test case for the situation and a better fix in #13271. Could you verify this? Many thanks.

@ligurio
Copy link
Contributor Author

ligurio commented Apr 3, 2024

Could you verify this? Many thanks.

Sure. I can confirm that this version fixes the problem.

@icing
Copy link
Contributor

icing commented Apr 3, 2024

Thanks, with the new test case in place, I hope we can avoid stumbling into this again in the future.

@bagder bagder closed this as completed in cfc65fd Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@icing icing

Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@icing @ligurio