Multiple calls to curl_multi_remove_handle with same easy handle no longer returns CURLM_OK

[seragh]

I did this

I have some code which calls curl_multi_remove_handle on application shutdown on an easy handle that might have already been removed on successfully doing it's work. Guess the original authors intention was to make sure curl gets shutdown cleanly irrespective.

Reproducer:

int main() {
    CURLM* mhandle = curl_multi_init();
    CURL* ehandle = curl_easy_init();

    curl_multi_add_handle(mhandle, ehandle);
    CURLMcode code1 = curl_multi_remove_handle(mhandle, ehandle);
    if (code1 != CURLM_OK) {
        printf("Failed first with %d\n", code1);
        return 1;
    }
    CURLMcode code2 = curl_multi_remove_handle(mhandle, ehandle);
    if (code2 != CURLM_OK) {
        printf("Failed second with %d\n", code2);
        return 1;
    }

    curl_easy_cleanup(ehandle);
    curl_multi_cleanup(mhandle);
}

Fails with "Failed second with 2", where in the past the second invocation returned CURLM_OK;

Bisecting points to ba235ab introducing this change.

I expected the following

Strictly speaking this could be considered a regression, but maybe you want to call it a legit bug fix. Let me know what your take is so I can decide how to go about it.

curl/libcurl version

8.10+

operating system

Linux, Debian (original reporter) and Gentoo (me)

[icing]

There can certainly be opinions about this. I think failing a remove for an already removed handle is the better way.

[jay]

In my opinion this is a bug. libcurl's return code for this situation varies depending on whether or not other easy handles are in the multi aka multi->num_easy:

curl/lib/multi.c

Lines 789 to 795 in 75a2079

	/* Verify that we got a somewhat good easy handle too */
	if(!GOOD_EASY_HANDLE(data) || !multi->num_easy)
	return CURLM_BAD_EASY_HANDLE;
	/* Prevent users from trying to remove same easy handle more than once */
	if(!data->multi)
	return CURLM_OK; /* it is already removed so let's say it is fine! */

I propose restoring the old behavior, see #15852.

[icing]

I change my mind. Removing an already removed handle should be ok.