Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cf-socket: don't bypass fclosesocket callback if cancelled before connect #11439

Closed
wants to merge 1 commit into from

Conversation

cgpe-a
Copy link
Contributor

@cgpe-a cgpe-a commented Jul 14, 2023

After upgrading to 8.1.2 from 7.84.0, I found that sockets were being closed without calling the fclosesocket callback if a request was cancelled after the associated socket was created, but before the socket was connected. This lead to an imbalance of fopensocket & fclosesocket callbacks, causing problems with a custom event loop integration using the multi-API.

This was caused by cf_socket_close() calling sclose() directly instead of calling socket_close() if the socket was not active. For regular TCP client connections, the socket is activated by cf_socket_active(), which is only called when the socket completes the connect.

As far as I can tell, this issue has existed since 7.88.0. That is, since the code in question was introduced by:
commit 71b7e01
Author: Stefan Eissing stefan@eissing.org
Date: Fri Dec 30 09:14:55 2022 +0100

    lib: connect/h2/h3 refactor

…nect

After upgrading to 8.1.2 from 7.84.0, I found that sockets were being
closed without calling the fclosesocket callback if a request was
cancelled after the associated socket was created, but before the socket
was connected. This lead to an imbalance of fopensocket & fclosesocket
callbacks, causing problems with a custom event loop integration using
the multi-API.

This was caused by cf_socket_close() calling sclose() directly instead
of calling socket_close() if the socket was not active. For regular TCP
client connections, the socket is activated by cf_socket_active(), which
is only called when the socket completes the connect.

As far as I can tell, this issue has existed since 7.88.0. That is,
since the code in question was introduced by:
    commit 71b7e01
    Author: Stefan Eissing <stefan@eissing.org>
    Date:   Fri Dec 30 09:14:55 2022 +0100

        lib: connect/h2/h3 refactor
@bagder
Copy link
Member

bagder commented Jul 14, 2023

Thanks!

@bagder bagder closed this in a70d97c Jul 14, 2023
@@ -871,7 +871,7 @@ static void cf_socket_close(struct Curl_cfilter *cf, struct Curl_easy *data)
/* this is our local socket, we did never publish it */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that comment still make sense? @icing @bagder

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure.

bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023
…nect

After upgrading to 8.1.2 from 7.84.0, I found that sockets were being
closed without calling the fclosesocket callback if a request was
cancelled after the associated socket was created, but before the socket
was connected. This lead to an imbalance of fopensocket & fclosesocket
callbacks, causing problems with a custom event loop integration using
the multi-API.

This was caused by cf_socket_close() calling sclose() directly instead
of calling socket_close() if the socket was not active. For regular TCP
client connections, the socket is activated by cf_socket_active(), which
is only called when the socket completes the connect.

As far as I can tell, this issue has existed since 7.88.0. That is,
since the code in question was introduced by:
    commit 71b7e01
    Author: Stefan Eissing <stefan@eissing.org>
    Date:   Fri Dec 30 09:14:55 2022 +0100

        lib: connect/h2/h3 refactor

Closes curl#11439
ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023
…nect

After upgrading to 8.1.2 from 7.84.0, I found that sockets were being
closed without calling the fclosesocket callback if a request was
cancelled after the associated socket was created, but before the socket
was connected. This lead to an imbalance of fopensocket & fclosesocket
callbacks, causing problems with a custom event loop integration using
the multi-API.

This was caused by cf_socket_close() calling sclose() directly instead
of calling socket_close() if the socket was not active. For regular TCP
client connections, the socket is activated by cf_socket_active(), which
is only called when the socket completes the connect.

As far as I can tell, this issue has existed since 7.88.0. That is,
since the code in question was introduced by:
    commit 71b7e01
    Author: Stefan Eissing <stefan@eissing.org>
    Date:   Fri Dec 30 09:14:55 2022 +0100

        lib: connect/h2/h3 refactor

Closes curl#11439
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants