url: close TLS before removing conn from cache

[chris-araman]

Fixes: #3412 #3505

Ensures any TLS shutdown messages are sent before removing the association between the connection and the easy handle. Reverts @bagder's previous partial fix for #3412.

Note that conn_free also calls Curl_ssl_close on PRIMARYSOCKET and SECONDARYSOCKET, but that was the case before fb445a1.

[chris-araman]

Thanks!

[jay]

Thanks. As you figured out Curl_ssl_close may write to the sockets and does in schannel and so expects at various points conn->data.

Note that conn_free also calls Curl_ssl_close on PRIMARYSOCKET and SECONDARYSOCKET

I'm interested in why that is, it seems more appropriate for disconnect. It's commented:

curl/lib/url.c

Lines 671 to 674 in 927a5bd

	/* close the SSL stuff before we close any sockets since they will/may
	write to the sockets */
	Curl_ssl_close(conn, FIRSTSOCKET);
	Curl_ssl_close(conn, SECONDARYSOCKET);

Currently though there is no guarantee of conn->data so if a shutdown needs to send or receive (like schannel) we run into the same problem if it's NULL. conn_free (as of your change) is no longer called in a way I can see that happening. Regardless I added a DEBUGASSERT(data) to the shutdown for schannel. I wonder what the downside would be to removing the ssl closes in conn_free, aren't they more correct just in the disconnect @bagder?