[prev in list] [next in list] [prev in thread] [next in thread] 

List:       apr-cvs
Subject:    svn commit: r1867226 - in /apr/apr/trunk: CHANGES include/apr_network_io.h network_io/win32/sendrecv
From:       ivan () apache ! org
Date:       2019-09-20 13:24:56
Message-ID: 20190920132457.555663A015F () svn01-us-west ! apache ! org
[Download RAW message or body]

Author: ivan
Date: Fri Sep 20 13:24:56 2019
New Revision: 1867226

URL: http://svn.apache.org/viewvc?rev=1867226&view=rev
Log:
Remove the APR_SENDFILE_DISCONNECT_SOCKET flag.

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. With this flag specified, the socket descriptor is removed from the
   apr_socket_t and the caller caller is expected to maintain the descriptor
   and release it manually, which is particularly error-prone.

See also:
https://lists.apache.org/thread.html/a3c4a03961a4b5842e7f657a15fe777edf5949b768a2807619a104b1@%3Cdev.apr.apache.org%3E



Modified:
    apr/apr/trunk/CHANGES
    apr/apr/trunk/include/apr_network_io.h
    apr/apr/trunk/network_io/win32/sendrecv.c

Modified: apr/apr/trunk/CHANGES
URL: http://svn.apache.org/viewvc/apr/apr/trunk/CHANGES?rev=1867226&r1=1867225&r2=1867226&view=diff
 ==============================================================================
--- apr/apr/trunk/CHANGES [utf-8] (original)
+++ apr/apr/trunk/CHANGES [utf-8] Fri Sep 20 13:24:56 2019
@@ -224,6 +224,8 @@ Changes for APR 2.0.0
   *) apr_socket_listen: Allow larger listen backlog values on Windows 8+.
      [Evgeny Kotkov <evgeny.kotkov visualsvn.com>]
 
+  *) Remove the 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

Modified: apr/apr/trunk/include/apr_network_io.h
URL: http://svn.apache.org/viewvc/apr/apr/trunk/include/apr_network_io.h?rev=1867226&r1=1867225&r2=1867226&view=diff
 ==============================================================================
--- apr/apr/trunk/include/apr_network_io.h (original)
+++ apr/apr/trunk/include/apr_network_io.h Fri Sep 20 13:24:56 2019
@@ -281,12 +281,7 @@ struct apr_sockaddr_t {
 };
 
 #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 */

Modified: apr/apr/trunk/network_io/win32/sendrecv.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/network_io/win32/sendrecv.c?rev=1867226&r1=1867225&r2=1867226&view=diff
 ==============================================================================
--- apr/apr/trunk/network_io/win32/sendrecv.c (original)
+++ apr/apr/trunk/network_io/win32/sendrecv.c Fri Sep 20 13:24:56 2019
@@ -267,7 +267,6 @@ APR_DECLARE(apr_status_t) apr_socket_sen
     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;
@@ -388,13 +387,6 @@ APR_DECLARE(apr_status_t) apr_socket_sen
                     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,21 +411,19 @@ APR_DECLARE(apr_status_t) apr_socket_sen
                                                  ? 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) {
@@ -473,17 +463,6 @@ APR_DECLARE(apr_status_t) apr_socket_sen
                 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;


[prev in list] [next in list] [prev in thread] [next in thread]