multi: fix incorrect multi_easy logic

[jay]

• Use data->multi and not data->multi_easy to refer to the active multi.

Prior to this change there were a few places that incorrectly treated data->multi_easy, if != NULL, as the easy handle's active multi.

Background:

The easy handle's active multi is always data->multi. It points to either the user-created multi (if the multi interface is used) or the internally created multi (if the easy interface is used). The internally created multi is always data->multi_easy and is retained until the easy handle is cleaned up.

If an easy handle is used with the easy interface and then the multi interface, data->multi_easy and data->multi will both exist and be different.

Closes #xxxxx

---

There's one other that I didn't change because I'm not sure if the multi_easy should be checked before multi:

curl/lib/connect.c

Lines 270 to 287 in 8edcfed

	/* this works for an easy handle:
	* - that has been used for curl_easy_perform()
	* - that is associated with a multi handle, and whose connection
	* was detached with CURLOPT_CONNECT_ONLY
	*/
	if((data->state.lastconnect_id != -1) && (data->multi_easy || data->multi)) {
	struct connectdata *c;
	struct connfind find;
	find.id_tofind = data->state.lastconnect_id;
	find.found = NULL;
	Curl_conncache_foreach(data,
	data->share && (data->share->specifier
	& (1<< CURL_LOCK_DATA_CONNECT))?
	&data->share->conn_cache:
	data->multi_easy?
	&data->multi_easy->conn_cache:
	&data->multi->conn_cache, &find, conn_is_conn);

Some logic is used to get what should be the connection cache used by the easy handle. I notice data->state.conn_cache is not checked so I assume it's not available at that point.

[bagder]

👍

[bagder]

@jay why not merge this?

[jay]

why not merge this?

I was unsure about Curl_getconnectinfo but I think I figured it out. Please review the squashme commit.

[bagder]

Is there actually a time when &data->multi->conn_cache is the incorrect choice here?

[bagder]

I mean, it can hardly reach this code with data->multi being NULL, can it ?

[bagder]

Why this change? If the easy interface is not used, the multi_easy pointer is NULL so isn't both the old and the new code achieving the same thing?

[jay]

If the easy interface is not used, the multi_easy pointer is NULL

sometimes. the rub is if an easy handle is used with the easy interface and then the multi interface. in that case they point to different multis. this pr addresses that issue by prioritizing multi over multi_easy because the former is always the in-use multi. my hang up with Curl_getconnectinfo (which is used in this logic) is, following this scenario, what happens when the easy handle is removed by the user via curl_multi_remove_handle, because that would seem to cause disconnection of connections in multi_easy and not multi. (edit: no, it disconnects the multi connections without affecting the multi_easy connections) afaict, this change fixes a bug by checking that they are different and Curl_getconnectinfo prioritizes multi over multi_easy

[bagder]

@jay I propose #12992 so that each easy handle only has one multi handle to relate to.

[bagder]

Now merged. With this, an easy handle can only ever be associated with a single multi handle.

[bagder]

This can now be adjusted

[bagder]

@jay do you want me to continue this?

I believe Curl_close() can also be simplified somewhat (when the easy handle is found to be part of a multi handle).

[jay]

do you want me to continue this?

I believe Curl_close() can also be simplified somewhat (when the easy handle is found to be part of a multi handle).

I dropped the fixup commit and changed the other commit's message, if more needs to be done have at it!

[jay]

@bagder can you have another look, I don't think there's anything left to add

[bagder]

Looks good!