Skip to content
Permalink
Browse files

pipelining: removed

As previously planned and documented in DEPRECATE.md, all pipelining
code is removed.

Closes #3651
  • Loading branch information...
bagder committed Apr 5, 2019
1 parent aba1c51 commit 2f44e94efb3df8e50bb2ddbc4ec6b569a6424517
@@ -5,46 +5,6 @@ email the curl-library mailing list as soon as possible and explain to us why
this is a problem for you and how your use case can't be satisfied properly
using a work around.

## HTTP pipelining

HTTP pipelining is badly supported by curl in the sense that we have bugs and
it is a fragile feature without enough tests. Also, when something turns out
to have problems it is really tricky to debug due to the timing sensitivity so
very often enabling debug outputs or similar completely changes the nature of
the behavior and things are not reproducing anymore!

HTTP pipelining was never enabled by default by the large desktop browsers due
to all the issues with it. Both Firefox and Chrome have also dropped
pipelining support entirely since a long time back now. We are in fact over
time becoming more and more lonely in supporting pipelining.

The bad state of HTTP pipelining was a primary driving factor behind HTTP/2
and its multiplexing feature. HTTP/2 multiplexing is truly and really
"pipelining done right". It is way more solid, practical and solves the use
case in a better way with better performance and fewer downsides and problems.

In 2018, pipelining *should* be abandoned and HTTP/2 should be used instead.

### State

In 7.62.0, we will add code that ignores the "enable pipeline" option
setting). The *setopt() function would still return "OK" though so the
application couldn't tell that this is happening.
Users who truly need pipelining from that version will need to modify the code
(ever so slightly) and rebuild.
### Removal
Six months later, in sync with the planned release happen in April 2019,
(might be 7.66.0), assuming no major riots have occurred due to this in the
mean time, we rip out the pipelining code. It is in the order of 1000 lines of
libcurl code.
Left to answer: should the *setopt() function start to return error when these
options are set to be able to tell when they're trying to use options that are
no longer around or should we maintain behavior as much as possible?
## `CURLOPT_DNS_USE_GLOBAL_CACHE`

This option makes libcurl use a global non-thread-safe cache for DNS if
@@ -5,7 +5,7 @@
.\" * | (__| |_| | _ <| |___
.\" * \___|\___/|_| \_\_____|
.\" *
.\" * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" *
.\" * This software is licensed as described in the file COPYING, which
.\" * you should have received as part of this distribution. The terms
@@ -28,6 +28,8 @@ CURLMOPT_CHUNK_LENGTH_PENALTY_SIZE \- chunk length threshold for pipelining

CURLMcode curl_multi_setopt(CURLM *handle, CURLMOPT_CHUNK_LENGTH_PENALTY_SIZE, long size);
.SH DESCRIPTION
No function since pipelining was removed in 7.62.0.

Pass a long with a \fBsize\fP in bytes. If a pipelined connection is currently
processing a chunked (Transfer-encoding: chunked) request with a current chunk
length larger than \fICURLMOPT_CHUNK_LENGTH_PENALTY_SIZE(3)\fP, that pipeline
@@ -5,7 +5,7 @@
.\" * | (__| |_| | _ <| |___
.\" * \___|\___/|_| \_\_____|
.\" *
.\" * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" *
.\" * This software is licensed as described in the file COPYING, which
.\" * you should have received as part of this distribution. The terms
@@ -28,6 +28,8 @@ CURLMOPT_CONTENT_LENGTH_PENALTY_SIZE \- size threshold for pipelining penalty

CURLMcode curl_multi_setopt(CURLM *handle, CURLMOPT_CONTENT_LENGTH_PENALTY_SIZE, long size);
.SH DESCRIPTION
No function since pipelining was removed in 7.62.0.

Pass a long with a \fBsize\fP in bytes. If a pipelined connection is currently
processing a request with a Content-Length larger than this
\fICURLMOPT_CONTENT_LENGTH_PENALTY_SIZE(3)\fP, that pipeline will then not be
@@ -5,7 +5,7 @@
.\" * | (__| |_| | _ <| |___
.\" * \___|\___/|_| \_\_____|
.\" *
.\" * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" *
.\" * This software is licensed as described in the file COPYING, which
.\" * you should have received as part of this distribution. The terms
@@ -28,6 +28,8 @@ CURLMOPT_MAX_PIPELINE_LENGTH \- maximum number of requests in a pipeline

CURLMcode curl_multi_setopt(CURLM *handle, CURLMOPT_MAX_PIPELINE_LENGTH, long max);
.SH DESCRIPTION
No function since pipelining was removed in 7.62.0.

Pass a long. The set \fBmax\fP number will be used as the maximum amount of
outstanding requests in an HTTP/1.1 pipelined connection. This option is only
used for HTTP/1.1 pipelining, not for HTTP/2 multiplexing.
@@ -5,7 +5,7 @@
.\" * | (__| |_| | _ <| |___
.\" * \___|\___/|_| \_\_____|
.\" *
.\" * Copyright (C) 1998 - 2018, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" *
.\" * This software is licensed as described in the file COPYING, which
.\" * you should have received as part of this distribution. The terms
@@ -71,12 +71,12 @@ HTTP(S)
.SH EXAMPLE
.nf
CURLM *m = curl_multi_init();
/* try HTTP/1 pipelining and HTTP/2 multiplexing */
curl_multi_setopt(m, CURLMOPT_PIPELINING, CURLPIPE_HTTP1 |
CURLPIPE_MULTIPLEX);
/* try HTTP/2 multiplexing */
curl_multi_setopt(m, CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX);
.fi
.SH AVAILABILITY
Added in 7.16.0. Multiplex support bit added in 7.43.0.
Added in 7.16.0. Multiplex support bit added in 7.43.0. HTTP/1 Pipelining
support was disabled in 7.62.0.
.SH RETURN VALUE
Returns CURLM_OK if the option is supported, and CURLM_UNKNOWN_OPTION if not.
.SH "SEE ALSO"
@@ -5,7 +5,7 @@
.\" * | (__| |_| | _ <| |___
.\" * \___|\___/|_| \_\_____|
.\" *
.\" * Copyright (C) 1998 - 2014, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" *
.\" * This software is licensed as described in the file COPYING, which
.\" * you should have received as part of this distribution. The terms
@@ -28,6 +28,8 @@ CURLMOPT_PIPELINING_SERVER_BL \- pipelining server blacklist

CURLMcode curl_multi_setopt(CURLM *handle, CURLMOPT_PIPELINING_SERVER_BL, char **servers);
.SH DESCRIPTION
No function since pipelining was removed in 7.62.0.

Pass a \fBservers\fP array of char *, ending with a NULL entry. This is a list
of server types prefixes (in the Server: HTTP header) that are blacklisted
from pipelining, i.e server types that are known to not support HTTP
@@ -5,7 +5,7 @@
.\" * | (__| |_| | _ <| |___
.\" * \___|\___/|_| \_\_____|
.\" *
.\" * Copyright (C) 1998 - 2014, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" *
.\" * This software is licensed as described in the file COPYING, which
.\" * you should have received as part of this distribution. The terms
@@ -28,6 +28,8 @@ CURLMOPT_PIPELINING_SITE_BL \- pipelining host blacklist

CURLMcode curl_multi_setopt(CURLM *handle, CURLMOPT_PIPELINING_SITE_BL, char **hosts);
.SH DESCRIPTION
No function since pipelining was removed in 7.62.0.

Pass a \fBhosts\fP array of char *, ending with a NULL entry. This is a list
of sites that are blacklisted from pipelining, i.e sites that are known to not
support HTTP pipelining. The array is copied by libcurl.
@@ -52,7 +52,7 @@ LIB_CFILES = file.c timeval.c base64.c hostip.c progress.c formdata.c \
openldap.c curl_gethostname.c gopher.c idn_win32.c \
http_proxy.c non-ascii.c asyn-ares.c asyn-thread.c curl_gssapi.c \
http_ntlm.c curl_ntlm_wb.c curl_ntlm_core.c curl_sasl.c rand.c \
curl_multibyte.c hostcheck.c conncache.c pipeline.c dotdot.c \
curl_multibyte.c hostcheck.c conncache.c dotdot.c \
x509asn1.c http2.c smb.c curl_endian.c curl_des.c system_win32.c \
mime.c sha256.c setopt.c curl_path.c curl_ctype.c curl_range.c psl.c \
doh.c urlapi.c altsvc.c
@@ -72,7 +72,7 @@ LIB_HFILES = arpa_telnet.h netrc.h file.h timeval.h hostip.h progress.h \
curl_gethostname.h gopher.h http_proxy.h non-ascii.h asyn.h \
http_ntlm.h curl_gssapi.h curl_ntlm_wb.h curl_ntlm_core.h \
curl_sasl.h curl_multibyte.h hostcheck.h conncache.h \
curl_setup_once.h multihandle.h setup-vms.h pipeline.h dotdot.h \
curl_setup_once.h multihandle.h setup-vms.h dotdot.h \
x509asn1.h http2.h sigpipe.h smb.h curl_endian.h curl_des.h \
curl_printf.h system_win32.h rand.h mime.h curl_sha256.h setopt.h \
curl_path.h curl_ctype.h curl_range.h psl.h doh.h urlapi-int.h \
@@ -7,7 +7,7 @@
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
* Copyright (C) 2015 - 2018, Daniel Stenberg, <daniel@haxx.se>, et al.
* Copyright (C) 2015 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
* Copyright (C) 2012 - 2014, Linus Nielsen Feltzing, <linus@haxx.se>
*
* This software is licensed as described in the file COPYING, which
@@ -40,7 +40,6 @@ struct conncache {

#define BUNDLE_NO_MULTIUSE -1
#define BUNDLE_UNKNOWN 0 /* initial value */
#define BUNDLE_PIPELINING 1
#define BUNDLE_MULTIPLEX 2

struct connectbundle {
@@ -73,7 +73,6 @@
#include "http_proxy.h"
#include "warnless.h"
#include "non-ascii.h"
#include "pipeline.h"
#include "http2.h"
#include "connect.h"
#include "strdup.h"
@@ -1280,7 +1279,6 @@ CURLcode Curl_add_buffer_send(Curl_send_buffer **inp,
This needs FIXing.
*/
return CURLE_SEND_ERROR;
Curl_pipeline_leave_write(conn);
}
}
Curl_add_buffer_free(&in);
@@ -3722,16 +3720,9 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data,
}
else if(conn->httpversion >= 11 &&
!conn->bits.close) {
/* If HTTP version is >= 1.1 and connection is persistent
server supports pipelining. */
/* If HTTP version is >= 1.1 and connection is persistent */
DEBUGF(infof(data,
"HTTP 1.1 or later with persistent connection, "
"pipelining supported\n"));
/* Activate pipelining if needed */
if(conn->bundle) {
if(!Curl_pipeline_site_blacklisted(data, conn))
conn->bundle->multiuse = BUNDLE_PIPELINING;
}
"HTTP 1.1 or later with persistent connection\n"));
}

switch(k->httpcode) {
@@ -3816,19 +3807,6 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data,
data->info.contenttype = contenttype;
}
}
else if(checkprefix("Server:", k->p)) {
if(conn->httpversion < 20) {
/* only do this for non-h2 servers */
char *server_name = Curl_copy_header_value(k->p);

/* Turn off pipelining if the server version is blacklisted */
if(conn->bundle && (conn->bundle->multiuse == BUNDLE_PIPELINING)) {
if(Curl_pipeline_server_blacklisted(data, server_name))
conn->bundle->multiuse = BUNDLE_NO_MULTIUSE;
}
free(server_name);
}
}
else if((conn->httpversion == 10) &&
conn->bits.httpproxy &&
Curl_compareheader(k->p,
@@ -620,7 +620,7 @@ static int push_promise(struct Curl_easy *data,

/*
* multi_connchanged() is called to tell that there is a connection in
* this multi handle that has changed state (pipelining become possible, the
* this multi handle that has changed state (multiplexing become possible, the
* number of allowed streams changed or similar), and a subsequent use of this
* multi handle should move CONNECT_PEND handles back to CONNECT to have them
* retry.
Oops, something went wrong.

3 comments on commit 2f44e94

@rouault

This comment has been minimized.

Copy link
Contributor

replied Apr 7, 2019

this commit causes #3745

@h1z1

This comment has been minimized.

Copy link

replied Apr 17, 2019

As a tool used for everything from troubleshooting to simple downloads, I'm floored at the breakneck speed in which this was done. Do you know for a fact pipeline is not used or are you basising this purely on browser use. You reference it was mentioned in your own internal deprecation schedule, I'd hope that was communicated at least to upstream distros?

@bagder

This comment has been minimized.

Copy link
Member Author

replied Apr 17, 2019

breakneck speed

I think we then have different opinions of what "breakneck speed" is. This was communicated clearly and in with details over six months ago and it has since then been clearly documented to happen - in the source code repository and on the web site. It has actually even been discussed on and off before that. If this surprises someone, that someone has not even tried to follow the curl project recently.

Do you know for a fact pipeline is not used or are you basising this purely on browser use.

I know for a fact that it is not used a lot with curl. I can be sure of this since we know the pipelining code had a few rather nasty known issues and yet hardly anyone reported problems with those. Or any other problems that relate to pipelining. All indications that it isn't a much used feature.

Additionally, we've been ready to keep the support in curl if anyone, and let me emphasize that: anyone, would be willing to step forward and get the implementation into decent shape by at least fixing the known problems and adding a few more tests. Not a single person has volunteered even though this bad situation has existed for a very long time.

Removing pipelining from curl is done for many separate reasons.

I'd hope that was communicated at least to upstream distros?

If they care about curl and curl development, they participate in the community and thus they know this via the same channels all other people and organizations interested in curl got the information (and for all I know, lots of distro people are!). We don't have any secret back channels to "talk to distros". I don't even know what such talks would entail or how to do them. Besides, what should these distros do? It's not like we ask them when we add things, when we fix things, when we improve things so why should we ask them for some kind of approval if we clean up our own backyard?

If you have further comments or suggestions on how we deprecate things in the project, about pipelining or how we communicate or inform the surrounding world of what we do, please take that discussion to the curl-library mailing list. Not by commenting to old commits that won't reach very many people.

Please sign in to comment.
You can’t perform that action at this time.