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

curl: fix --upload-file . hangs if delay in STDIN #4599

Closed
wants to merge 1 commit into from
Closed

curl: fix --upload-file . hangs if delay in STDIN #4599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions docs/libcurl/opts/CURLOPT_PROGRESSFUNCTION.3
View file
Open in desktop
Expand Up @@ -60,8 +60,11 @@ Unknown/unused argument values passed to the callback will be set to zero
the callback will be called one or more times first, before it knows the data
sizes so a program must be made to handle that.

Returning a non-zero value from this callback will cause libcurl to abort the
transfer and return \fICURLE_ABORTED_BY_CALLBACK\fP.
If your callback function returns CURL_PROGRESSFUNC_CONTINUE it will cause
libcurl to continue executing the default progress function.

Returning any other non-zero value from this callback will cause libcurl to
abort the transfer and return \fICURLE_ABORTED_BY_CALLBACK\fP.

If you transfer data with the multi interface, this function will not be
called during periods of idleness unless you call the appropriate libcurl
Expand Down
7 changes: 5 additions & 2 deletions docs/libcurl/opts/CURLOPT_XFERINFOFUNCTION.3
View file
Open in desktop
Expand Up @@ -57,8 +57,11 @@ Unknown/unused argument values passed to the callback will be set to zero
the callback will be called one or more times first, before it knows the data
sizes so a program must be made to handle that.

Returning a non-zero value from this callback will cause libcurl to abort the
transfer and return \fICURLE_ABORTED_BY_CALLBACK\fP.
If your callback function returns CURL_PROGRESSFUNC_CONTINUE it will cause
libcurl to continue executing the default progress function.

Returning any other non-zero value from this callback will cause libcurl to
abort the transfer and return \fICURLE_ABORTED_BY_CALLBACK\fP.

If you transfer data with the multi interface, this function will not be
called during periods of idleness unless you call the appropriate libcurl
Expand Down
1 change: 1 addition & 0 deletions docs/libcurl/symbols-in-versions
View file
Open in desktop
Expand Up @@ -867,6 +867,7 @@ CURL_POLL_INOUT 7.14.0
CURL_POLL_NONE 7.14.0
CURL_POLL_OUT 7.14.0
CURL_POLL_REMOVE 7.14.0
CURL_PROGRESSFUNC_CONTINUE 7.68.0
CURL_PROGRESS_BAR 7.1.1 - 7.4.1
CURL_PROGRESS_STATS 7.1.1 - 7.4.1
CURL_PUSH_DENY 7.44.0
Expand Down
5 changes: 5 additions & 0 deletions include/curl/curl.h
View file
Open in desktop
Expand Up @@ -209,6 +209,11 @@ struct curl_httppost {
set. Added in 7.46.0 */
};


/* This is a return code for the progress callback that, when returned, will
signal libcurl to continue executing the default progress function */
#define CURL_PROGRESSFUNC_CONTINUE 0x10000001

/* This is the CURLOPT_PROGRESSFUNCTION callback prototype. It is now
considered deprecated but was the only choice up until 7.31.0 */
typedef int (*curl_progress_callback)(void *clientp,
Expand Down
18 changes: 11 additions & 7 deletions lib/progress.c
View file
Open in desktop
Expand Up @@ -594,11 +594,13 @@ int Curl_pgrsUpdate(struct connectdata *conn)
data->progress.size_ul,
data->progress.uploaded);
Curl_set_in_callback(data, false);
if(result)
failf(data, "Callback aborted");
return result;
if(result != CURL_PROGRESSFUNC_CONTINUE) {
if(result)
failf(data, "Callback aborted");
return result;
}
}
if(data->set.fprogress) {
else if(data->set.fprogress) {
int result;
/* The older deprecated callback is set, call that */
Curl_set_in_callback(data, true);
Expand All @@ -608,9 +610,11 @@ int Curl_pgrsUpdate(struct connectdata *conn)
(double)data->progress.size_ul,
(double)data->progress.uploaded);
Curl_set_in_callback(data, false);
if(result)
failf(data, "Callback aborted");
return result;
if(result != CURL_PROGRESSFUNC_CONTINUE) {
if(result)
failf(data, "Callback aborted");
return result;
}
}

if(showprogress)
Expand Down
11 changes: 10 additions & 1 deletion src/tool_cb_prg.c
View file
Open in desktop
Expand Up @@ -32,6 +32,7 @@
#include "tool_cfgable.h"
#include "tool_cb_prg.h"
#include "tool_util.h"
#include "tool_operate.h"

#include "memdebug.h" /* keep this as LAST include */

Expand Down Expand Up @@ -121,7 +122,10 @@ int tool_progress_cb(void *clientp,
and this new edition inherits some of his concepts. */

struct timeval now = tvnow();
struct ProgressData *bar = (struct ProgressData *)clientp;
struct per_transfer *per = clientp;
struct OutStruct *outs = &per->outs;
struct OperationConfig *config = outs->config;
struct ProgressData *bar = &per->progressbar;
curl_off_t total;
curl_off_t point;

Expand Down Expand Up @@ -191,6 +195,11 @@ int tool_progress_cb(void *clientp,
bar->prev = point;
bar->prevtime = now;

if(config->readbusy) {
config->readbusy = FALSE;
curl_easy_pause(per->curl, CURLPAUSE_CONT);
}

return 0;
}

Expand Down
26 changes: 26 additions & 0 deletions src/tool_cb_rea.c
View file
Open in desktop
Expand Up @@ -27,6 +27,7 @@

#include "tool_cfgable.h"
#include "tool_cb_rea.h"
#include "tool_operate.h"

#include "memdebug.h" /* keep this as LAST include */

Expand All @@ -52,3 +53,28 @@ size_t tool_read_cb(void *buffer, size_t sz, size_t nmemb, void *userdata)
in->config->readbusy = FALSE;
return (size_t)rc;
}

/*
** callback for CURLOPT_XFERINFOFUNCTION used to unpause busy reads
*/

int tool_readbusy_cb(void *clientp,
curl_off_t dltotal, curl_off_t dlnow,
curl_off_t ultotal, curl_off_t ulnow)
{
struct per_transfer *per = clientp;
struct OutStruct *outs = &per->outs;
struct OperationConfig *config = outs->config;

(void)dltotal; /* unused */
(void)dlnow; /* unused */
(void)ultotal; /* unused */
(void)ulnow; /* unused */

if(config->readbusy) {
config->readbusy = FALSE;
curl_easy_pause(per->curl, CURLPAUSE_CONT);
}

return per->noprogress? 0 : CURL_PROGRESSFUNC_CONTINUE;
}
8 changes: 8 additions & 0 deletions src/tool_cb_rea.h
View file
Open in desktop
Expand Up @@ -29,4 +29,12 @@

size_t tool_read_cb(void *buffer, size_t sz, size_t nmemb, void *userdata);

/*
** callback for CURLOPT_XFERINFOFUNCTION used to unpause busy reads
*/

int tool_readbusy_cb(void *clientp,
curl_off_t dltotal, curl_off_t dlnow,
curl_off_t ultotal, curl_off_t ulnow);

#endif /* HEADER_CURL_TOOL_CB_REA_H */
13 changes: 10 additions & 3 deletions src/tool_operate.c
View file
Open in desktop
Expand Up @@ -1080,11 +1080,11 @@ static CURLcode single_transfer(struct GlobalConfig *global,
isatty(fileno(outs->stream)))
/* we send the output to a tty, therefore we switch off the progress
meter */
global->noprogress = global->isatty = TRUE;
per->noprogress = global->noprogress = global->isatty = TRUE;
else {
/* progress meter is per download, so restore config
values */
global->noprogress = orig_noprogress;
per->noprogress = global->noprogress = orig_noprogress;
global->isatty = orig_isatty;
}

Expand Down Expand Up @@ -1573,7 +1573,14 @@ static CURLcode single_transfer(struct GlobalConfig *global,
/* we want the alternative style, then we have to implement it
ourselves! */
my_setopt(curl, CURLOPT_XFERINFOFUNCTION, tool_progress_cb);
my_setopt(curl, CURLOPT_XFERINFODATA, &per->progressbar);
my_setopt(curl, CURLOPT_XFERINFODATA, per);
}
else if(per->uploadfile && !strcmp(per->uploadfile, ".")) {
/* when reading from stdin in non-blocking mode, we use the progress
function to unpause a busy read */
my_setopt(curl, CURLOPT_NOPROGRESS, 0L);
my_setopt(curl, CURLOPT_XFERINFOFUNCTION, tool_readbusy_cb);
my_setopt(curl, CURLOPT_XFERINFODATA, per);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not happy with how this will

  1. disable the regular default progress bar
  2. disable the --progress-bar option if set
  3. Not work for parallel transfers

... but I also don't have a very good answer on how to do this instead! 😢

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I definitely agree. I'm happy to explore other approaches, but I couldn't think of a better way. I wanted to put this out there to see what you guys thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding 2 above, It will actually honor the --progress-bar option. I just pushed an update that adds the same busy read checking into the regular CURLOPT_XFERINFOFUNCTION handler also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding 3 above. I added the busy read checking to the parallel CURLOPT_XFERINFOFUNCTION handler also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding 1 above. I added a return code to the progress callback that will cause libcurl to continue executing the default progress function.

}

/* new in libcurl 7.24.0: */
Expand Down
1 change: 1 addition & 0 deletions src/tool_operate.h
View file
Open in desktop
Expand Up @@ -44,6 +44,7 @@ struct per_transfer {
char *outfile;
bool infdopen; /* TRUE if infd needs closing */
int infd;
bool noprogress;
struct ProgressData progressbar;
struct OutStruct outs;
struct OutStruct heads;
Expand Down
8 changes: 8 additions & 0 deletions src/tool_progress.c
View file
Open in desktop
Expand Up @@ -95,10 +95,18 @@ int xferinfo_cb(void *clientp,
curl_off_t ulnow)
{
struct per_transfer *per = clientp;
struct OutStruct *outs = &per->outs;
struct OperationConfig *config = outs->config;
per->dltotal = dltotal;
per->dlnow = dlnow;
per->ultotal = ultotal;
per->ulnow = ulnow;

if(config->readbusy) {
config->readbusy = FALSE;
curl_easy_pause(per->curl, CURLPAUSE_CONT);
}

return 0;
}

Expand Down