Navigation Menu

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

libcurl: stop reading from connection when client has paused receivin… #3240

Closed
wants to merge 1 commit into from
Closed

Conversation

dheerajsangamkar
Copy link

…g data

Goal: Allow libcurl pause-unpause behavior to control flow of data from server with limited memory consumption.

Setup: Using curl_multi_perform to transfer GBs of data from a server that is setup using a curl_easy_handle. (HTTP 1.1 and HTTP 2) When it is not possible to accept data, the WRITEFUNCTION returns CURL_WRITEFUNC_PAUSE. Curl stops sending data to the write function.

Problem:
Use curl_easy_pause(easy_handle, CURLPAUSE_CONT) in the main thread to 'unpause' the transfer that was paused earlier. The un-paused transfer starts and calls the WRITEFUNC again and after a few invocation gets CURL_WRITEFUNC_PAUSE. At this point, libcurl continues to read data from the connection and has to store it off of the easy handle because it cannot deliver the data to the WRITEFUNCTION that has paused. When reading GBs of data, this ultimately leads to Out of memory error from CURL and the transfer is aborted.

Root cause, in my opinion:
curl_multi_perform/curl_easy_pause ultimately calls lib/transfer.c:readwrite_data to read data off the connection from http server and delivers to WRITEFUNCTION using Curl_client_write. When WRITEFUNC requests pause, this state is set in the easy-handle but readwrite_data does not check this state and continues to read data from socket but cannot deliver to the WRITEFUNC.

Solution:
readwrite_data should check the KEEP_RECV_PAUSE flag in the easy handle and stop reading when it is set.

@dheerajsangamkar
Copy link
Author

Related conversation on curl-library email list: https://curl.haxx.se/mail/lib-2018-11/0007.html

Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can only agree that this appears to be completely sensible. It's also a bit complicated to add a test for, so while the green CI tests are a good sign we also know that we don't have any tests for this particular code path.

@bagder
Copy link
Member

bagder commented Nov 6, 2018

Thanks!

@bagder bagder closed this in 74f4782 Nov 6, 2018
@dheerajsangamkar
Copy link
Author

Thanks!

@lock lock bot locked as resolved and limited conversation to collaborators Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants