Hi! The apr_socket_sendfile() has Windows specific flag APR_SENDFILE_DISCONNECT_SOCKET. When this flag specified instructs Winsock to disconnect socket and prepare it for reuse after file send.
There are several problems with this flag: 1. The TCP socket may be subject to the TCP TIME_WAIT state. That means apr_sock_sendfile() could occupy worker thread for significant time (30 seconds) 2. When flag specified socket descriptor is removed from apr_socket_t and will not be cleaned up. The caller expected to maintain original socket descriptor and release if APR_SENDFILE_DISCONNECT_SOCKET is used. As far I see Apache HTTP Server doesn't use APR_SENDFILE_DISCONNECT_SOCKET flag. As far I understand proper way to reuse sockets is to use DisconnectEx() [1]. Application should use DisconnectEx() in overlapped I/O mode and wait for it completion using completion port or thread pool. When overlapped I/O is done the socket could be added to reuse list and later used for AcceptEx() or ConnectEx(). So I propose to remove APR_SENDFILE_DISCONNECT_SOCKET flag in APR 2.0 for the reasons above. Patch is attached. Any objections? [1] https://docs.microsoft.com/en-gb/windows/win32/api/mswsock/nc-mswsock-lpfn_disconnectex -- Ivan Zhakov
Index: CHANGES =================================================================== --- CHANGES (revision 1866558) +++ CHANGES (working copy) @@ -224,6 +224,8 @@ *) apr_socket_listen: Allow larger listen backlog values on Windows 8+. [Evgeny Kotkov <evgeny.kotkov visualsvn.com>] + *) Windows platform: Remove APR_SENDFILE_DISCONNECT_SOCKET flag. [Ivan Zhakov] + Changes for APR and APR-util 1.6.x and later: *) http://svn.apache.org/viewvc/apr/apr/branches/1.6.x/CHANGES?view=markup Index: include/apr_network_io.h =================================================================== --- include/apr_network_io.h (revision 1866558) +++ include/apr_network_io.h (working copy) @@ -281,12 +281,7 @@ }; #if APR_HAS_SENDFILE -/** - * Support reusing the socket on platforms which support it (from disconnect, - * specifically Win32. - * @remark Optional flag passed into apr_socket_sendfile() - */ -#define APR_SENDFILE_DISCONNECT_SOCKET 1 +/* APR_SENDFILE_DISCONNECT_SOCKET has been removed in APR 2.0. */ #endif /** A structure to encapsulate headers and trailers for apr_socket_sendfile */ Index: network_io/win32/sendrecv.c =================================================================== --- network_io/win32/sendrecv.c (revision 1866934) +++ network_io/win32/sendrecv.c (working copy) @@ -267,7 +267,6 @@ apr_size_t nbytes; TRANSMIT_FILE_BUFFERS tfb, *ptfb = NULL; apr_size_t bytes_to_send; /* Bytes to send out of the file (not including headers) */ - int disconnected = 0; int sendv_trailers = 0; char hdtrbuf[4096]; LPFN_TRANSMITFILE pfn_transmit_file = NULL; @@ -274,6 +273,10 @@ static GUID wsaid_transmitfile = WSAID_TRANSMITFILE; DWORD dw; + if (flags & 1 /* APR_SENDFILE_DISCONNECT_SOCKET */) { + return APR_EINVAL; + } + /* According to documentation TransmitFile() should not be used directly. * Pointer to function should retrieved using WSAIoctl: * https://docs.microsoft.com/en-gb/windows/win32/api/mswsock/nf-mswsock-transmitfile#remarks @@ -388,13 +391,6 @@ sendv_trailers = 1; } } - /* Disconnect the socket after last send */ - if ((flags & APR_SENDFILE_DISCONNECT_SOCKET) - && !sendv_trailers) { - dwFlags |= TF_REUSE_SOCKET; - dwFlags |= TF_DISCONNECT; - disconnected = 1; - } } sock->overlapped->Offset = (DWORD)(curoff); @@ -419,22 +415,20 @@ ? sock->timeout_ms : INFINITE)); if (rv == WAIT_OBJECT_0) { status = APR_SUCCESS; - if (!disconnected) { - if (!WSAGetOverlappedResult(sock->socketdes, - sock->overlapped, - &xmitbytes, - FALSE, - &dwFlags)) { - status = apr_get_netos_error(); - } - /* Ugly code alert: WSAGetOverlappedResult returns - * a count of all bytes sent. This loop only - * tracks bytes sent out of the file. - */ - else if (ptfb) { - xmitbytes -= (ptfb->HeadLength + ptfb->TailLength); - } + if (!WSAGetOverlappedResult(sock->socketdes, + sock->overlapped, + &xmitbytes, + FALSE, + &dwFlags)) { + status = apr_get_netos_error(); } + /* Ugly code alert: WSAGetOverlappedResult returns + * a count of all bytes sent. This loop only + * tracks bytes sent out of the file. + */ + else if (ptfb) { + xmitbytes -= (ptfb->HeadLength + ptfb->TailLength); + } } else if (rv == WAIT_TIMEOUT) { status = APR_FROM_OS_ERROR(WAIT_TIMEOUT); @@ -473,17 +467,6 @@ return rv; *len += nbytes; } - - - /* Mark the socket as disconnected, but do not close it. - * Note: The application must have stored the socket prior to making - * the call to apr_socket_sendfile in order to either reuse it - * or close it. - */ - if (disconnected) { - sock->disconnected = 1; - sock->socketdes = INVALID_SOCKET; - } } return status;