diff --git a/buckets/apr_buckets_file.c b/buckets/apr_buckets_file.c index f0eab5a180..bba59c62fe 100644 --- a/buckets/apr_buckets_file.c +++ b/buckets/apr_buckets_file.c @@ -19,6 +19,10 @@ #include "apr_file_io.h" #include "apr_buckets.h" +#if APR_HAVE_UNISTD_H +#include /* for sysconf() */ +#endif + #if APR_HAS_MMAP #include "apr_mmap.h" @@ -70,6 +74,39 @@ static int file_make_mmap(apr_bucket *e, apr_size_t filelength, file_bucket_destroy(a); return 1; } + +/* Returns the number of bytes to read from the bucket to reach the + * next page boundary in the file, or zero if the bucket should not be + * split at a page boundary. A bucket which does not start on a page + * boundary, and which cannot be read in a single read regardless, is + * read up to the next page boundary so that the remainder of the + * bucket can be mapped from a page-aligned offset, provided that + * remainder is large enough to be mapped. */ +static apr_size_t file_align_length(apr_bucket_file *a, apr_size_t filelength, + apr_off_t fileoffset) +{ +#ifdef _SC_PAGESIZE + long psize; + + if (!a->can_mmap || filelength <= a->read_size) { + return 0; + } + + psize = sysconf(_SC_PAGESIZE); + if (psize > 0 && (fileoffset & (apr_off_t)(psize - 1)) != 0) { + apr_size_t align = (apr_size_t)(psize + - (fileoffset & (apr_off_t)(psize - 1))); + + /* Only worthwhile if enough of the bucket remains beyond the + * page boundary to be mapped. */ + if (align < filelength + && filelength - align >= APR_MMAP_THRESHOLD) { + return align; + } + } +#endif + return 0; +} #endif static apr_status_t file_bucket_read(apr_bucket *e, const char **str, @@ -87,7 +124,10 @@ static apr_status_t file_bucket_read(apr_bucket *e, const char **str, #endif #if APR_HAS_MMAP - if (file_make_mmap(e, filelength, fileoffset, a->readpool)) { + apr_size_t alignlength = file_align_length(a, filelength, fileoffset); + + if (!alignlength + && file_make_mmap(e, filelength, fileoffset, a->readpool)) { return apr_bucket_read(e, str, len, block); } #endif @@ -110,6 +150,11 @@ static apr_status_t file_bucket_read(apr_bucket *e, const char **str, *str = NULL; /* in case we die prematurely */ *len = (filelength > a->read_size) ? a->read_size : filelength; +#if APR_HAS_MMAP + if (alignlength && alignlength < *len) { + *len = alignlength; + } +#endif buf = apr_bucket_alloc(*len, e->list); /* Handle offset ... */ diff --git a/mmap/unix/mmap.c b/mmap/unix/mmap.c index 5f86dd5edc..48753266ab 100644 --- a/mmap/unix/mmap.c +++ b/mmap/unix/mmap.c @@ -92,7 +92,6 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap_t **new, if (file == NULL || file->filedes == -1 || file->buffered) return APR_EBADF; - (*new) = (apr_mmap_t *)apr_pcalloc(cont, sizeof(apr_mmap_t)); if (flag & APR_MMAP_WRITE) { native_flags |= PROT_WRITE; @@ -108,7 +107,6 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap_t **new, assert(psize > 0 && (psize & (psize - 1)) == 0); } poffset = offset & (apr_off_t)(psize - 1); - (*new)->poffset = poffset; #endif mm = mmap(NULL, size + poffset, @@ -121,9 +119,11 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap_t **new, return errno; } - mm = (char *)mm + poffset; - - (*new)->mm = mm; + /* Allocate only on success, so that a caller which repeatedly + * fails does not consume memory from the pool. */ + (*new) = (apr_mmap_t *)apr_pcalloc(cont, sizeof(apr_mmap_t)); + (*new)->poffset = poffset; + (*new)->mm = (char *)mm + poffset; (*new)->size = size; (*new)->cntxt = cont; APR_RING_ELEM_INIT(*new, link); diff --git a/mmap/win32/mmap.c b/mmap/win32/mmap.c index d6dbf790eb..5ff7b97d7f 100644 --- a/mmap/win32/mmap.c +++ b/mmap/win32/mmap.c @@ -77,6 +77,8 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap_t **new, apr_file_t *file, apr_size_t psize; apr_off_t poffset; DWORD offlo, offhi; + HANDLE mhandle; + void *mv; if (size == 0) return APR_EINVAL; @@ -102,7 +104,6 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap_t **new, apr_file_t *file, memblock = si.dwAllocationGranularity; } - *new = apr_pcalloc(cont, sizeof(apr_mmap_t)); pstart = (offset / memblock) * memblock; poffset = offset - pstart; psize = (apr_size_t)poffset + size; @@ -111,9 +112,8 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap_t **new, apr_file_t *file, * of the mapped region! */ - (*new)->mhandle = CreateFileMappingW(file->filehand, NULL, fmaccess, - 0, 0, NULL); - if (!(*new)->mhandle || (*new)->mhandle == INVALID_HANDLE_VALUE) + mhandle = CreateFileMappingW(file->filehand, NULL, fmaccess, 0, 0, NULL); + if (!mhandle || mhandle == INVALID_HANDLE_VALUE) { *new = NULL; return apr_get_os_error(); @@ -121,16 +121,20 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap_t **new, apr_file_t *file, offlo = (DWORD)pstart; offhi = (DWORD)(pstart >> 32); - (*new)->mv = MapViewOfFile((*new)->mhandle, mvaccess, offhi, - offlo, psize); - if (!(*new)->mv) + mv = MapViewOfFile(mhandle, mvaccess, offhi, offlo, psize); + if (!mv) { apr_status_t rv = apr_get_os_error(); - CloseHandle((*new)->mhandle); + CloseHandle(mhandle); *new = NULL; return rv; } + /* Allocate only on success, so that a caller which repeatedly + * fails does not consume memory from the pool. */ + *new = apr_pcalloc(cont, sizeof(apr_mmap_t)); + (*new)->mhandle = mhandle; + (*new)->mv = mv; (*new)->mm = (char *)(*new)->mv + poffset; (*new)->size = size; (*new)->cntxt = cont; diff --git a/test/testbuckets.c b/test/testbuckets.c index d882f87818..d5b8952f8e 100644 --- a/test/testbuckets.c +++ b/test/testbuckets.c @@ -555,6 +555,197 @@ static void test_truncfile(abts_case *tc, void *data) apr_bucket_alloc_destroy(ba); } +#if APR_HAS_MMAP +#include "apr_mmap.h" + +#if APR_HAVE_UNISTD_H +#include /* for sysconf() */ +#endif + +/* Byte expected at offset OFF of a file created by make_pattern_file(). */ +#define PATTERN_BYTE(off) ((char)((off) % 251)) + +/* Make a test file named FNAME of length LEN, opened with FLAGS. */ +static apr_file_t *make_pattern_file(abts_case *tc, const char *fname, + apr_off_t len, apr_int32_t flags) +{ + apr_file_t *f; + char buf[8192]; + apr_off_t off = 0; + apr_status_t rv; + + rv = apr_file_open(&f, fname, + APR_FOPEN_WRITE|APR_FOPEN_TRUNCATE|APR_FOPEN_CREATE, + APR_FPROT_OS_DEFAULT, p); + APR_ASSERT_SUCCESS(tc, "create test file", rv); + if (rv != APR_SUCCESS) { + return NULL; + } + + while (off < len) { + apr_size_t n, chunk = sizeof buf; + + if ((apr_off_t)chunk > len - off) { + chunk = (apr_size_t)(len - off); + } + for (n = 0; n < chunk; n++) { + buf[n] = PATTERN_BYTE(off + n); + } + APR_ASSERT_SUCCESS(tc, "write test file contents", + apr_file_write_full(f, buf, chunk, NULL)); + off += chunk; + } + apr_file_close(f); + + rv = apr_file_open(&f, fname, flags, APR_FPROT_OS_DEFAULT, p); + APR_ASSERT_SUCCESS(tc, "reopen test file", rv); + + return rv == APR_SUCCESS ? f : NULL; +} + +/* Returns non-zero if the LEN bytes at BUF match those expected at + * offset OFF of a file created by make_pattern_file(). */ +static int pattern_match(const char *buf, apr_size_t len, apr_off_t off) +{ + apr_size_t n; + + for (n = 0; n < len; n++) { + if (buf[n] != PATTERN_BYTE(off + n)) { + return 0; + } + } + return 1; +} + +/* A FILE bucket which starts at an offset which is not a multiple of + * the page size, and is larger than APR_MMAP_LIMIT, is read up to the + * next page boundary and then via a sequence of MMAP buckets. */ +static void test_mmap_unaligned(abts_case *tc, void *data) +{ + static const char fname[] = "mmapfile.bin"; + apr_bucket_alloc_t *ba = apr_bucket_alloc_create(p); + apr_bucket_brigade *bb = apr_brigade_create(p, ba); + const apr_off_t start = 4321; + const apr_size_t tail = 100000; + const apr_size_t flen = APR_MMAP_LIMIT + tail; + apr_size_t expect[4] = { 0 }; /* expected bucket lengths */ + int first_mmap = 0; /* index of first MMAP bucket */ + apr_off_t off = start; + apr_file_t *f; + apr_bucket *e; + int count = 0; + +#ifdef _SC_PAGESIZE + { + apr_size_t psize = (apr_size_t)sysconf(_SC_PAGESIZE); + + /* Assumes the page size is small enough that the boundary is + * reached in a single read. */ + if (psize > APR_BUCKET_BUFF_SIZE) { + ABTS_NOT_IMPL(tc, "Skipped: page size too large"); + return; + } + expect[count++] = psize - (apr_size_t)start % psize; + first_mmap = 1; + } +#endif + expect[count++] = APR_MMAP_LIMIT; + expect[count] = flen - APR_MMAP_LIMIT - (first_mmap ? expect[0] : 0); + + f = make_pattern_file(tc, fname, start + flen, APR_FOPEN_READ); + if (!f) { + return; + } + + apr_brigade_insert_file(bb, f, start, flen, p); + + count = 0; + while ((e = APR_BRIGADE_FIRST(bb)) != APR_BRIGADE_SENTINEL(bb) + && count < 3) { + const char *buf; + apr_size_t len; + + ABTS_ASSERT(tc, "bucket is a FILE bucket before read", + APR_BUCKET_IS_FILE(e)); + APR_ASSERT_SUCCESS(tc, "read file bucket", + apr_bucket_read(e, &buf, &len, APR_BLOCK_READ)); + if (count < first_mmap) { + ABTS_ASSERT(tc, "FILE bucket read up to page boundary", + !APR_BUCKET_IS_MMAP(e)); + } + else { + ABTS_ASSERT(tc, "FILE bucket morphed to MMAP bucket", + APR_BUCKET_IS_MMAP(e)); + } + ABTS_SIZE_EQUAL(tc, expect[count], len); + ABTS_ASSERT(tc, "bucket contents match", + pattern_match(buf, len, off)); + + off += len; + count++; + apr_bucket_delete(e); + } + + ABTS_ASSERT(tc, "read whole file", off == start + (apr_off_t)flen); + ABTS_ASSERT(tc, "brigade is empty", APR_BRIGADE_EMPTY(bb)); + + apr_file_close(f); + apr_brigade_destroy(bb); + apr_bucket_alloc_destroy(ba); + apr_file_remove(fname, p); +} + +/* If the file cannot be mmap()ed, the FILE bucket must fall back to + * reading the file. */ +static void test_mmap_fail(abts_case *tc, void *data) +{ + static const char fname[] = "mmapfile.bin"; + apr_bucket_alloc_t *ba = apr_bucket_alloc_create(p); + apr_bucket_brigade *bb = apr_brigade_create(p, ba); + const apr_size_t flen = 3 * APR_BUCKET_BUFF_SIZE; + apr_off_t off = 0; + apr_file_t *f; + apr_bucket *e; + const char *buf; + apr_size_t len; + + /* apr_mmap_create() fails for a buffered file. */ + f = make_pattern_file(tc, fname, flen, + APR_FOPEN_READ | APR_FOPEN_BUFFERED); + if (!f) { + return; + } + + e = apr_brigade_insert_file(bb, f, 0, flen, p); + + APR_ASSERT_SUCCESS(tc, "read file bucket", + apr_bucket_read(e, &buf, &len, APR_BLOCK_READ)); + ABTS_ASSERT(tc, "FILE bucket did not morph to MMAP bucket", + !APR_BUCKET_IS_MMAP(e)); + ABTS_ASSERT(tc, "partial read of file", len > 0 && len < flen); + + ABTS_ASSERT(tc, "remainder is a FILE bucket", + APR_BUCKET_NEXT(e) != APR_BRIGADE_SENTINEL(bb) + && APR_BUCKET_IS_FILE(APR_BUCKET_NEXT(e))); + + /* The whole file is still readable. */ + while ((e = APR_BRIGADE_FIRST(bb)) != APR_BRIGADE_SENTINEL(bb)) { + APR_ASSERT_SUCCESS(tc, "read bucket", + apr_bucket_read(e, &buf, &len, APR_BLOCK_READ)); + ABTS_ASSERT(tc, "bucket contents match", + pattern_match(buf, len, off)); + off += len; + apr_bucket_delete(e); + } + ABTS_ASSERT(tc, "read whole file", off == (apr_off_t)flen); + + apr_file_close(f); + apr_brigade_destroy(bb); + apr_bucket_alloc_destroy(ba); + apr_file_remove(fname, p); +} +#endif /* APR_HAS_MMAP */ + static const char hello[] = "hello, world"; static void test_partition(abts_case *tc, void *data) @@ -674,6 +865,10 @@ abts_suite *testbuckets(abts_suite *suite) abts_run_test(suite, test_insertfile, NULL); abts_run_test(suite, test_manyfile, NULL); abts_run_test(suite, test_truncfile, NULL); +#if APR_HAS_MMAP + abts_run_test(suite, test_mmap_unaligned, NULL); + abts_run_test(suite, test_mmap_fail, NULL); +#endif abts_run_test(suite, test_partition, NULL); abts_run_test(suite, test_write_split, NULL); abts_run_test(suite, test_write_putstrs, NULL); diff --git a/test/testmmap.c b/test/testmmap.c index fe21705c8d..2c10faddc7 100644 --- a/test/testmmap.c +++ b/test/testmmap.c @@ -257,6 +257,50 @@ static void test_mmap_offset(abts_case *tc, void *data) } +/* A failed apr_mmap_create() must not consume memory from the pool; + * callers such as the FILE bucket may retry many times within the + * lifetime of one pool. */ +static void test_mmap_create_fail(abts_case *tc, void *data) +{ + static const char fname[] = "data/mmap_wronly.txt"; + apr_pool_t *subp; + apr_file_t *f; + apr_mmap_t *mm = (void *)tc; /* any non-NULL value */ + apr_status_t rv; + apr_size_t before; + + apr_pool_create(&subp, ptest); + + /* A file which is open only for writing cannot be mapped for + * reading. */ + rv = apr_file_open(&f, fname, + APR_FOPEN_WRITE | APR_FOPEN_CREATE | APR_FOPEN_TRUNCATE, + APR_FPROT_OS_DEFAULT, subp); + APR_ASSERT_SUCCESS(tc, "open file for writing", rv); + if (rv != APR_SUCCESS) { + return; + } + APR_ASSERT_SUCCESS(tc, "write to file", apr_file_puts("hello, world", f)); + + /* apr_pool_num_bytes() always returns zero unless built with + * pool debugging, so this is only checked in such builds. */ + before = apr_pool_num_bytes(subp, 0); + + rv = apr_mmap_create(&mm, f, 0, 5, APR_MMAP_READ, subp); + ABTS_TRUE(tc, rv != APR_SUCCESS); + ABTS_PTR_EQUAL(tc, NULL, mm); + + ABTS_ASSERT(tc, "failed apr_mmap_create() allocated from the pool", + apr_pool_num_bytes(subp, 0) == before); + + if (mm) { + apr_mmap_delete(mm); + } + apr_file_close(f); + apr_file_remove(fname, subp); + apr_pool_destroy(subp); +} + #endif abts_suite *testmmap(abts_suite *suite) @@ -280,6 +324,7 @@ abts_suite *testmmap(abts_suite *suite) abts_run_test(suite, test_file_close, NULL); apr_pool_clear(ptest); } + abts_run_test(suite, test_mmap_create_fail, NULL); apr_pool_destroy(ptest); #else abts_run_test(suite, not_implemented, NULL);