From 7b9bc96c7716f34dbd1f525aefb77d74b8b0f528 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 17 Jul 2018 00:29:11 +0200 Subject: http2: several cleanups - separate easy handle from connections better - added asserts on a number of places - added sanity check of pipelines for debug builds Closes #2751 --- lib/hostasyn.c | 2 +- lib/http.c | 13 ++++++---- lib/http2.c | 9 +++++-- lib/http2.h | 4 +-- lib/pipeline.c | 6 ++--- lib/url.c | 77 ++++++++++++++++------------------------------------------ lib/urldata.h | 2 +- 7 files changed, 43 insertions(+), 70 deletions(-) diff --git a/lib/hostasyn.c b/lib/hostasyn.c index 35b52a90c..e7b399ed2 100644 --- a/lib/hostasyn.c +++ b/lib/hostasyn.c @@ -131,7 +131,7 @@ CURLcode Curl_async_resolved(struct connectdata *conn, if(result) /* We're not allowed to return failure with memory left allocated in the connectdata struct, free those here */ - Curl_disconnect(conn->data, conn, FALSE); /* close the connection */ + Curl_disconnect(conn->data, conn, TRUE); /* close the connection */ return result; } diff --git a/lib/http.c b/lib/http.c index 4ec5f2be6..9bbf59b79 100644 --- a/lib/http.c +++ b/lib/http.c @@ -158,18 +158,20 @@ CURLcode Curl_http_setup_conn(struct connectdata *conn) /* allocate the HTTP-specific struct for the Curl_easy, only to survive during this request */ struct HTTP *http; - DEBUGASSERT(conn->data->req.protop == NULL); + struct Curl_easy *data = conn->data; + DEBUGASSERT(data->req.protop == NULL); http = calloc(1, sizeof(struct HTTP)); if(!http) return CURLE_OUT_OF_MEMORY; Curl_mime_initpart(&http->form, conn->data); - conn->data->req.protop = http; - - Curl_http2_setup_conn(conn); - Curl_http2_setup_req(conn->data); + data->req.protop = http; + if(!CONN_INUSE(conn)) + /* if not alredy multi-using, setup connection details */ + Curl_http2_setup_conn(conn); + Curl_http2_setup_req(data); return CURLE_OK; } @@ -1913,6 +1915,7 @@ CURLcode Curl_http(struct connectdata *conn, bool *done) } http = data->req.protop; + DEBUGASSERT(http); if(!data->state.this_is_a_follow) { /* Free to avoid leaking memory on multiple requests*/ diff --git a/lib/http2.c b/lib/http2.c index 8251d96df..6511451af 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -154,6 +154,11 @@ static void http2_stream_free(struct HTTP *http) } } +/* + * Disconnects *a* connection used for HTTP/2. It might be an old one from the + * connection cache and not the "main" one. Don't touch the easy handle! + */ + static CURLcode http2_disconnect(struct connectdata *conn, bool dead_connection) { @@ -164,8 +169,6 @@ static CURLcode http2_disconnect(struct connectdata *conn, nghttp2_session_del(c->h2); Curl_safefree(c->inbuf); - http2_stream_free(conn->data->req.protop); - conn->data->state.drain = 0; H2BUGF(infof(conn->data, "HTTP/2 DISCONNECT done\n")); @@ -520,6 +523,7 @@ static int push_promise(struct Curl_easy *data, if(rv) { /* denied, kill off the new handle again */ http2_stream_free(newhandle->req.protop); + newhandle->req.protop = NULL; (void)Curl_close(newhandle); goto fail; } @@ -535,6 +539,7 @@ static int push_promise(struct Curl_easy *data, if(rc) { infof(data, "failed to add handle to multi\n"); http2_stream_free(newhandle->req.protop); + newhandle->req.protop = NULL; Curl_close(newhandle); rv = 1; goto fail; diff --git a/lib/http2.h b/lib/http2.h index f597c805e..21cd9b848 100644 --- a/lib/http2.h +++ b/lib/http2.h @@ -7,7 +7,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2017, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2018, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -65,7 +65,7 @@ void Curl_http2_cleanup_dependencies(struct Curl_easy *data); #define Curl_http2_request_upgrade(x,y) CURLE_UNSUPPORTED_PROTOCOL #define Curl_http2_setup(x) CURLE_UNSUPPORTED_PROTOCOL #define Curl_http2_switched(x,y,z) CURLE_UNSUPPORTED_PROTOCOL -#define Curl_http2_setup_conn(x) +#define Curl_http2_setup_conn(x) Curl_nop_stmt #define Curl_http2_setup_req(x) #define Curl_http2_init_state(x) #define Curl_http2_init_userset(x) diff --git a/lib/pipeline.c b/lib/pipeline.c index 068940920..8de3babd7 100644 --- a/lib/pipeline.c +++ b/lib/pipeline.c @@ -6,7 +6,7 @@ * \___|\___/|_| \_\_____| * * Copyright (C) 2013, Linus Nielsen Feltzing, - * Copyright (C) 2013 - 2017, Daniel Stenberg, , et al. + * Copyright (C) 2013 - 2018, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -110,8 +110,8 @@ CURLcode Curl_add_handle_to_pipeline(struct Curl_easy *handle, pipeline = &conn->send_pipe; result = addHandleToPipeline(handle, pipeline); - - if(pipeline == &conn->send_pipe && sendhead != conn->send_pipe.head) { + if((conn->bundle->multiuse == BUNDLE_PIPELINING) && + (pipeline == &conn->send_pipe && sendhead != conn->send_pipe.head)) { /* this is a new one as head, expire it */ Curl_pipeline_leave_write(conn); /* not in use yet */ Curl_expire(conn->send_pipe.head->ptr, 0, EXPIRE_RUN_NOW); diff --git a/lib/url.c b/lib/url.c index f5a5e9586..48d697b3c 100644 --- a/lib/url.c +++ b/lib/url.c @@ -127,7 +127,6 @@ bool curl_win32_idn_to_ascii(const char *in, char **out); static void conn_free(struct connectdata *conn); static void free_fixed_hostname(struct hostname *host); -static void signalPipeClose(struct curl_llist *pipeline, bool pipe_broke); static CURLcode parse_url_login(struct Curl_easy *data, struct connectdata *conn, char **userptr, char **passwdptr, @@ -750,7 +749,7 @@ CURLcode Curl_disconnect(struct Curl_easy *data, return CURLE_OK; /* this is closed and fine already */ if(!data) { - DEBUGF(fprintf(stderr, "DISCONNECT without easy handle, ignoring\n")); + DEBUGF(infof(data, "DISCONNECT without easy handle, ignoring\n")); return CURLE_OK; } @@ -758,9 +757,8 @@ CURLcode Curl_disconnect(struct Curl_easy *data, * If this connection isn't marked to force-close, leave it open if there * are other users of it */ - if(!conn->bits.close && CONN_INUSE(conn)) { - DEBUGF(fprintf(stderr, "Curl_disconnect when inuse: %d\n", - CONN_INUSE(conn))); + if(CONN_INUSE(conn) && !dead_connection) { + DEBUGF(infof(data, "Curl_disconnect when inuse: %d\n", CONN_INUSE(conn))); return CURLE_OK; } @@ -792,14 +790,7 @@ CURLcode Curl_disconnect(struct Curl_easy *data, Curl_ssl_close(conn, FIRSTSOCKET); - /* Indicate to all handles on the pipe that we're dead */ - if(Curl_pipeline_wanted(data->multi, CURLPIPE_ANY)) { - signalPipeClose(&conn->send_pipe, TRUE); - signalPipeClose(&conn->recv_pipe, TRUE); - } - conn_free(conn); - return CURLE_OK; } @@ -888,6 +879,16 @@ static void Curl_printPipeline(struct curl_llist *pipeline) static struct Curl_easy* gethandleathead(struct curl_llist *pipeline) { struct curl_llist_element *curr = pipeline->head; +#ifdef DEBUGBUILD + { + struct curl_llist_element *p = pipeline->head; + while(p) { + struct Curl_easy *e = p->ptr; + DEBUGASSERT(GOOD_EASY_HANDLE(e)); + p = p->next; + } + } +#endif if(curr) { return (struct Curl_easy *) curr->ptr; } @@ -920,33 +921,6 @@ void Curl_getoff_all_pipelines(struct Curl_easy *data, } } -static void signalPipeClose(struct curl_llist *pipeline, bool pipe_broke) -{ - struct curl_llist_element *curr; - - if(!pipeline) - return; - - curr = pipeline->head; - while(curr) { - struct curl_llist_element *next = curr->next; - struct Curl_easy *data = (struct Curl_easy *) curr->ptr; - -#ifdef DEBUGBUILD /* debug-only code */ - if(data->magic != CURLEASY_MAGIC_NUMBER) { - /* MAJOR BADNESS */ - infof(data, "signalPipeClose() found BAAD easy handle\n"); - } -#endif - - if(pipe_broke) - data->state.pipe_broke = TRUE; - Curl_multi_handlePipeBreak(data); - Curl_llist_remove(pipeline, curr, NULL); - curr = next; - } -} - static bool proxy_info_matches(const struct proxy_info* data, const struct proxy_info* needle) @@ -2498,18 +2472,6 @@ static CURLcode setup_connection_internals(struct connectdata *conn) { const struct Curl_handler * p; CURLcode result; - struct Curl_easy *data = conn->data; - - /* in some case in the multi state-machine, we go back to the CONNECT state - and then a second (or third or...) call to this function will be made - without doing a DISCONNECT or DONE in between (since the connection is - yet in place) and therefore this function needs to first make sure - there's no lingering previous data allocated. */ - Curl_free_request_state(data); - - memset(&data->req, 0, sizeof(struct SingleRequest)); - data->req.maxdownload = -1; - conn->socktype = SOCK_STREAM; /* most of them are TCP streams */ /* Perform setup complement if some. */ @@ -4675,12 +4637,16 @@ CURLcode Curl_connect(struct Curl_easy *data, *asyncp = FALSE; /* assume synchronous resolves by default */ + /* init the single-transfer specific data */ + Curl_free_request_state(data); + memset(&data->req, 0, sizeof(struct SingleRequest)); + data->req.maxdownload = -1; + /* call the stuff that needs to be called */ result = create_conn(data, in_connect, asyncp); if(!result) { - /* no error */ - if((*in_connect)->send_pipe.size || (*in_connect)->recv_pipe.size) + if(CONN_INUSE(*in_connect)) /* pipelining */ *protocol_done = TRUE; else if(!*asyncp) { @@ -4695,11 +4661,10 @@ CURLcode Curl_connect(struct Curl_easy *data, *in_connect = NULL; return result; } - - if(result && *in_connect) { + else if(result && *in_connect) { /* We're not allowed to return failure with memory left allocated in the connectdata struct, free those here */ - Curl_disconnect(data, *in_connect, FALSE); + Curl_disconnect(data, *in_connect, TRUE); *in_connect = NULL; /* return a NULL */ } diff --git a/lib/urldata.h b/lib/urldata.h index 07eb48869..7d396f3f2 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -786,7 +786,7 @@ struct connectdata { other easy handle without careful consideration (== only for pipelining/multiplexing) and it cannot be used by another multi handle! */ -#define CONN_INUSE(c) ((c)->send_pipe.size || (c)->recv_pipe.size) +#define CONN_INUSE(c) ((c)->send_pipe.size + (c)->recv_pipe.size) /**** Fields set when inited and not modified again */ long connection_id; /* Contains a unique number to make it easier to -- cgit v1.2.3