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;
