Skip to content

socket: use accept4 when available #16979

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

Closed
wants to merge 3 commits into from
Closed

Conversation

panjf2000
Copy link
Contributor

@panjf2000 panjf2000 commented Apr 5, 2025

Linux, *BSD, and Solaris support accept4 system call that enables the caller to assign additional flags and save some extra system calls.
It can come in handy when O_NONBLOCK or/and FD_CLOEXEC is/are required on a socket after being accepted.

Ref:
https://man7.org/linux/man-pages/man2/accept.2.html https://man.freebsd.org/cgi/man.cgi?query=accept4
https://man.dragonflybsd.org/?command=accept&section=2 https://man.openbsd.org/accept.2
https://man.netbsd.org/accept.2
https://docs.oracle.com/cd/E88353_01/html/E37843/accept4-3c.html
https://www.gnu.org/software/gnulib/manual/html_node/accept4.html

@panjf2000 panjf2000 force-pushed the accept4 branch 3 times, most recently from 1504d22 to 745d7e4 Compare April 5, 2025 16:54
@testclutch
Copy link

Analysis of PR #16979 at 745d7e4f:

Test 101 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Test 103 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Test 108 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Test 119 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Test 144 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Test 145 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Test 212 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

There are more failures, but that's enough from Gha.

Generated by Testclutch

Linux, *BSD, and Solaris support accept4 system call
that enables the caller to assign additional flags and
save some extra system calls.
It can come in handy when O_NONBLOCK or/and FD_CLOEXEC
is/are required on a socket after being accepted.

Ref:
https://man7.org/linux/man-pages/man2/accept.2.html
https://man.freebsd.org/cgi/man.cgi?query=accept4
https://man.dragonflybsd.org/?command=accept&section=2
https://man.openbsd.org/accept.2
https://man.netbsd.org/accept.2
https://docs.oracle.com/cd/E88353_01/html/E37843/accept4-3c.html
https://www.gnu.org/software/gnulib/manual/html_node/accept4.html
@panjf2000
Copy link
Contributor Author

This is weird, I passed all the tests on my Linux server:

image

Anything special on those failed CIs? @bagder @icing @vszakats

@dfandrich
Copy link
Contributor

The failing tests all seem to have debug mode enabled. Can you reproduce the failures if you configure with --enable-debug?

@panjf2000
Copy link
Contributor Author

The failing tests all seem to have debug mode enabled. Can you reproduce the failures if you configure with --enable-debug?

image

@panjf2000
Copy link
Contributor Author

Some failed tests arose, but they don't seem to be the same as those previous ones on CI's.

@bagder
Copy link
Member

bagder commented Apr 6, 2025

Add accept4 support to memdebug:

From a212acb9f2cfe740529aa03cd4fb173ec176367e Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Sun, 6 Apr 2025 11:21:42 +0200
Subject: [PATCH] memdebug: add accept4

---
 lib/memdebug.c | 18 ++++++++++++++++++
 lib/memdebug.h |  6 ++++++
 2 files changed, 24 insertions(+)

diff --git a/lib/memdebug.c b/lib/memdebug.c
index 58e4614f5f..b351726b32 100644
--- a/lib/memdebug.c
+++ b/lib/memdebug.c
@@ -376,10 +376,28 @@ curl_socket_t curl_dbg_accept(curl_socket_t s, void *saddr, void *saddrlen,
                  source, line, sockfd);
 
   return sockfd;
 }
 
+#ifdef HAVE_ACCEPT4
+curl_socket_t curl_dbg_accept4(curl_socket_t s, void *saddr, void *saddrlen,
+                               int flags,
+                               int line, const char *source)
+{
+  struct sockaddr *addr = (struct sockaddr *)saddr;
+  curl_socklen_t *addrlen = (curl_socklen_t *)saddrlen;
+
+  curl_socket_t sockfd = accept4(s, addr, addrlen, flags);
+
+  if(source && (sockfd != CURL_SOCKET_BAD))
+    curl_dbg_log("FD %s:%d accept() = %" FMT_SOCKET_T "\n",
+                 source, line, sockfd);
+
+  return sockfd;
+}
+#endif
+
 /* separate function to allow libcurl to mark a "faked" close */
 void curl_dbg_mark_sclose(curl_socket_t sockfd, int line, const char *source)
 {
   if(source)
     curl_dbg_log("FD %s:%d sclose(%" FMT_SOCKET_T ")\n",
diff --git a/lib/memdebug.h b/lib/memdebug.h
index aabcaadef4..fbea0c38bd 100644
--- a/lib/memdebug.h
+++ b/lib/memdebug.h
@@ -90,10 +90,13 @@ CURL_EXTERN void curl_dbg_mark_sclose(curl_socket_t sockfd,
                                       int line, const char *source);
 CURL_EXTERN int curl_dbg_sclose(curl_socket_t sockfd,
                                 int line, const char *source);
 CURL_EXTERN curl_socket_t curl_dbg_accept(curl_socket_t s, void *a, void *alen,
                                           int line, const char *source);
+CURL_EXTERN curl_socket_t curl_dbg_accept4(curl_socket_t s, void *saddr,
+                                           void *saddrlen, int flags,
+                                           int line, const char *source);
 #ifdef HAVE_SOCKETPAIR
 CURL_EXTERN int curl_dbg_socketpair(int domain, int type, int protocol,
                                     curl_socket_t socket_vector[2],
                                     int line, const char *source);
 #endif
@@ -157,10 +160,13 @@ CURL_EXTERN ALLOC_FUNC
 #define socket(domain,type,protocol)\
  curl_dbg_socket((int)domain, type, protocol, __LINE__, __FILE__)
 #undef accept /* for those with accept as a macro */
 #define accept(sock,addr,len)\
  curl_dbg_accept(sock, addr, len, __LINE__, __FILE__)
+#undef accept4 /* for those with accept as a macro */
+#define accept4(sock,addr,len,flags)                            \
+  curl_dbg_accept4(sock, addr, len, flags, __LINE__, __FILE__)
 #ifdef HAVE_SOCKETPAIR
 #define socketpair(domain,type,protocol,socket_vector)\
  curl_dbg_socketpair((int)domain, type, protocol, socket_vector, \
                      __LINE__, __FILE__)
 #endif
-- 
2.49.0

@panjf2000
Copy link
Contributor Author

Any chance that this failed CI is a flaky one and is irrelevant to this PR?

@bagder
Copy link
Member

bagder commented Apr 6, 2025

Any chance that this failed CI is a flaky one and is irrelevant to this PR?

I am pretty sure that it is and is not result of your work.

@panjf2000
Copy link
Contributor Author

Alright, I think we are good to go! Could you take a glance at this? @bagder

@bagder bagder closed this in 3d02872 Apr 6, 2025
@bagder
Copy link
Member

bagder commented Apr 6, 2025

Thanks!

@panjf2000 panjf2000 deleted the accept4 branch April 6, 2025 12:38
@Myriachan
Copy link
Contributor

Myriachan commented Apr 8, 2025

On Windows, you can get the atomic equivalent of O_CLOEXEC by calling WSASocketW() with WSA_FLAG_NO_HANDLE_INHERIT | WSA_FLAG_OVERLAPPED. (Note that socket() uses WSA_FLAG_OVERLAPPED internally, so this flag is needed if using WSASocketW() instead.)

WSA_FLAG_NO_HANDLE_INHERIT requires Windows 8.0+ though, so if you need 7 compatibility it'll have to detect versions.

WinSock's accept() copies the no-inherit flag from the listening socket, so there's no direct equivalent of accept4() needed.

nbaws pushed a commit to nbaws/curl that referenced this pull request Apr 26, 2025
Linux, *BSD, and Solaris support accept4 system call that enables the
caller to assign additional flags and save some extra system calls. It
can come in handy when O_NONBLOCK or/and FD_CLOEXEC is/are required on a
socket after being accepted.

Ref:
https://man7.org/linux/man-pages/man2/accept.2.html
https://man.freebsd.org/cgi/man.cgi?query=accept4
https://man.dragonflybsd.org/?command=accept&section=2
https://man.openbsd.org/accept.2
https://man.netbsd.org/accept.2
https://docs.oracle.com/cd/E88353_01/html/E37843/accept4-3c.html
https://www.gnu.org/software/gnulib/manual/html_node/accept4.html

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

Successfully merging this pull request may close these issues.

6 participants