url: change conn shutdown order to unlink data as last step

- Split off connection shutdown procedure from Curl_disconnect into new
  function conn_shutdown.

- Change the shutdown procedure to close the sockets before
  disassociating the transfer.

Prior to this change the sockets were closed after disassociating the
transfer so SOCKETFUNCTION wasn't called since the transfer was already
disassociated. That likely came about from recent work started in
Jan 2019 (#3442) to separate transfers from connections.

Bug: https://curl.haxx.se/mail/lib-2019-02/0101.html
Reported-by: Pavel Löbl

Closes https://github.com/curl/curl/issues/3597
Closes https://github.com/curl/curl/pull/3598

1 files changed, 24 insertions, 15 deletions

diff --git a/lib/url.c b/lib/url.c
index 61971a4ea..ecabe491f 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -660,11 +660,15 @@ static void conn_reset_all_postponed_data(struct connectdata *conn)
 #define conn_reset_all_postponed_data(c) do {} WHILE_FALSE
 #endif /* ! USE_RECV_BEFORE_SEND_WORKAROUND */
 
-static void conn_free(struct connectdata *conn)
+
+static void conn_shutdown(struct connectdata *conn)
 {
   if(!conn)
     return;
 
+  infof(conn->data, "Closing connection %ld\n", conn->connection_id);
+  DEBUGASSERT(conn->data);
+
   /* possible left-overs from the async name resolvers */
   Curl_resolver_cancel(conn);
 
@@ -688,6 +692,21 @@ static void conn_free(struct connectdata *conn)
   Curl_ntlm_wb_cleanup(conn);
 #endif
 
+  /* unlink ourselves. this should be called last since other shutdown
+     procedures need a valid conn->data and this may clear it. */
+  Curl_conncache_remove_conn(conn->data, conn, TRUE);
+}
+
+static void conn_free(struct connectdata *conn)
+{
+  if(!conn)
+    return;
+
+  free_idnconverted_hostname(&conn->host);
+  free_idnconverted_hostname(&conn->conn_to_host);
+  free_idnconverted_hostname(&conn->http_proxy.host);
+  free_idnconverted_hostname(&conn->socks_proxy.host);
+
   Curl_safefree(conn->user);
   Curl_safefree(conn->passwd);
   Curl_safefree(conn->oauth_bearer);
@@ -781,25 +800,15 @@ CURLcode Curl_disconnect(struct Curl_easy *data,
   Curl_http_ntlm_cleanup(conn);
 #endif
 
-  /* the protocol specific disconnect handler needs a transfer for its
-     connection! */
+  /* the protocol specific disconnect handler and conn_shutdown need a transfer
+     for the connection! */
   conn->data = data;
+
   if(conn->handler->disconnect)
     /* This is set if protocol-specific cleanups should be made */
     conn->handler->disconnect(conn, dead_connection);
 
-  infof(data, "Closing connection %ld\n", conn->connection_id);
-  Curl_ssl_close(conn, FIRSTSOCKET);
-  Curl_ssl_close(conn, SECONDARYSOCKET);
-
-  /* unlink ourselves! */
-  Curl_conncache_remove_conn(data, conn, TRUE);
-
-  free_idnconverted_hostname(&conn->host);
-  free_idnconverted_hostname(&conn->conn_to_host);
-  free_idnconverted_hostname(&conn->http_proxy.host);
-  free_idnconverted_hostname(&conn->socks_proxy.host);
-
+  conn_shutdown(conn);
   conn_free(conn);
   return CURLE_OK;
 }