Jim Jagielski
Wed, 07 May 2008 10:40:41 -0700
On May 7, 2008, at 1:29 PM, Jim Jagielski wrote:
Squashed... Spent a LOT of time on this (even ported the Linux impl to Darwin, which actually is nicer since it uses apr_socket_sendv() and does things like checking partial header/footer stuff, which I may eventually commit to trunk) and the fix was simple...
Here is the super-revised implementation: Index: network_io/unix/sendrecv.c =================================================================== --- network_io/unix/sendrecv.c (revision 654186) +++ network_io/unix/sendrecv.c (working copy) @@ -406,15 +406,15 @@ #elif defined(DARWIN) /* OS/X Release 10.5 or greater */-apr_status_t apr_socket_sendfile(apr_socket_t * sock, apr_file_t * file,
- apr_hdtr_t * hdtr, apr_off_t * offset,
- apr_size_t * len, apr_int32_t flags)
+apr_status_t apr_socket_sendfile(apr_socket_t *sock, apr_file_t *file,
+ apr_hdtr_t *hdtr, apr_off_t *offset,
+ apr_size_t *len, apr_int32_t flags)
{
apr_off_t nbytes = 0;
apr_off_t bytes_to_send = *len;
- apr_size_t header_bytes_written = 0;
+ apr_off_t bytes_sent = 0;
+ apr_status_t arv;
int rv = 0;
- int sent_headers = 0;
/* Ignore flags for now. */
flags = 0;
@@ -425,8 +425,31 @@
/* OS X can send the headers/footers as part of the system call,
* but how it counts bytes isn't documented properly. We use
- * writev() instead.
+ * apr_socket_sendv() instead.
*/
+ if (hdtr->numheaders > 0) {
+ apr_size_t hbytes;
+ int i;
+
+ /* Now write the headers */
+ arv = apr_socket_sendv(sock, hdtr->headers, hdtr->numheaders,
+ &hbytes);
+ if (arv != APR_SUCCESS) {
+ *len = 0;
+ return errno;
+ }
+ bytes_sent = hbytes;
+
+ hbytes = 0;
+ for (i = 0; i < hdtr->numheaders; i++) {
+ hbytes += hdtr->headers[i].iov_len;
+ }
+ if (bytes_sent < hbytes) {
+ *len = bytes_sent;
+ return APR_SUCCESS;
+ }
+ }
+
do {
if (sock->options & APR_INCOMPLETE_WRITE) {
apr_status_t arv;
@@ -438,72 +461,40 @@
}
}
- if (!sent_headers) {
- if (hdtr->numheaders) {
- rv = writev(sock->socketdes,
- hdtr->headers,
- hdtr->numheaders);
- if (rv > 0) {
- header_bytes_written = rv;
- sent_headers = 1;
- rv = 0;
- }
- }
- else {
- sent_headers = 1;
- }
- }
+ nbytes = bytes_to_send;
+ rv = sendfile(file->filedes, /* file to be sent */
+ sock->socketdes, /* socket */
+ *offset, /* where in the file to start */
+ &nbytes, /* number of bytes to write/
written */
+ NULL, /* Headers/footers */
+ flags); /* undefined, set to 0 */
- if (bytes_to_send && sent_headers) {
- /* We won't dare call sendfile() if we don't have
- * header or file bytes to send because nbytes == 0
- * means send the remaining file to EOF.
- */
- nbytes = bytes_to_send;
- rv = sendfile(file->filedes, /* file to be sent */
- sock->socketdes, /* socket */
- *offset, /* where in the file to
start */
- &nbytes, /* number of bytes to write/
written */
- NULL, /* Headers/footers */
- flags); /* undefined, set to 0 */
-
- bytes_to_send -= nbytes;
- if (rv == -1) {
- if (errno == EAGAIN) {
- if (sock->timeout > 0) {
- sock->options |= APR_INCOMPLETE_WRITE;
- }
- /* BSD's sendfile can return -1/EAGAIN even if it
- * sent bytes. Sanitize the result so we get
normal EAGAIN
- * semantics w.r.t. bytes sent.
- */
- else if (nbytes) {
- /* normal exit for a big file & non-blocking
io */
- (*len) = nbytes + header_bytes_written;
- return APR_SUCCESS;
- }
+ bytes_sent += nbytes;
+ bytes_to_send -= nbytes;
+ (*offset) += nbytes;
+ if (rv == -1) {
+ if (errno == EAGAIN) {
+ if (sock->timeout > 0) {
+ sock->options |= APR_INCOMPLETE_WRITE;
}
- }
- else { /* rv == 0 (or the kernel is broken) */
- if (nbytes == 0) {
- /* Most likely the file got smaller after the stat.
- * Return an error so the caller can do the Right
Thing.
- */ - (*len) = nbytes + header_bytes_written; - return APR_EOF; + /* BSD's sendfile can return -1/EAGAIN even if it+ * sent bytes. Sanitize the result so we get normal EAGAIN
+ * semantics w.r.t. bytes sent.
+ */
+ else if (nbytes) {
+ /* normal exit for a big file & non-blocking io */
+ (*len) = bytes_sent;
+ return APR_SUCCESS;
}
}
}
-
- if (sent_headers && !bytes_to_send) {
- /* just trailer bytes... use writev()
- */
- rv = writev(sock->socketdes,
- hdtr->trailers,
- hdtr->numtrailers);
- if (rv > 0) {
- nbytes += rv;
- rv = 0;
+ else { /* rv == 0 (or the kernel is broken) */
+ if (nbytes == 0) {
+ /* Most likely the file got smaller after the stat.
+ * Return an error so the caller can do the Right
Thing.
+ */
+ (*len) = bytes_sent;
+ return APR_EOF;
}
}
@@ -512,7 +503,20 @@
}
} while (rv == -1 && (errno == EINTR || errno == EAGAIN));
- (*len) = nbytes + header_bytes_written;
+ /* Now write the footers */
+ if (hdtr->numtrailers > 0) {
+ apr_size_t tbytes;
+ arv = apr_socket_sendv(sock, hdtr->trailers, hdtr->numtrailers,
+ &tbytes);
+ bytes_sent += tbytes;
+ if (arv != APR_SUCCESS) {
+ *len = bytes_sent;
+ rv = errno;
+ return rv;
+ }
+ }
+
+ (*len) = bytes_sent;
if (rv == -1) {
return errno;
}