From 19c844594e40d79cea016b54f9ab3a367440b4c9 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 21 Sep 2020 13:57:05 +0200 Subject: [PATCH] Fix mmap copying Instead of attempting to map large files into memory at once, we map chunks of at most `PHP_STREAM_MMAP_MAX` bytes, and repeat that until we hit the point where `php_stream_seek()` fails (see bug 54902), and copy the rest of the file by reading and writing small chunks. We also fix the mapping behavior for zero bytes on Windows, which did not error (as with `mmap()`), but would have mapped the remaining file. --- NEWS | 4 +++ main/streams/php_stream_mmap.h | 2 ++ main/streams/plain_wrapper.c | 5 +++ main/streams/streams.c | 59 ++++++++++++++++++++++------------ 4 files changed, 50 insertions(+), 20 deletions(-) diff --git a/NEWS b/NEWS index 7ad5262506d..4259d4d0e8a 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? 2020, PHP 7.4.12 +- Core: + . Fixed bug #80061 (Copying large files may have suboptimal performance). + (cmb) + - MySQLnd: . Fixed bug #80115 (mysqlnd.debug doesn't recognize absolute paths with slashes). (cmb) diff --git a/main/streams/php_stream_mmap.h b/main/streams/php_stream_mmap.h index 132f3214c11..40288cc27f2 100644 --- a/main/streams/php_stream_mmap.h +++ b/main/streams/php_stream_mmap.h @@ -58,6 +58,8 @@ typedef struct { #define PHP_STREAM_MMAP_ALL 0 +#define PHP_STREAM_MMAP_MAX (512 * 1024 * 1024) + #define php_stream_mmap_supported(stream) (_php_stream_set_option((stream), PHP_STREAM_OPTION_MMAP_API, PHP_STREAM_MMAP_SUPPORTED, NULL) == 0 ? 1 : 0) /* Returns 1 if the stream in its current state can be memory mapped, diff --git a/main/streams/plain_wrapper.c b/main/streams/plain_wrapper.c index 50ca925dea3..298950cf9ed 100644 --- a/main/streams/plain_wrapper.c +++ b/main/streams/plain_wrapper.c @@ -826,6 +826,11 @@ static int php_stdiop_set_option(php_stream *stream, int option, int value, void delta = (DWORD)range->offset - loffs; } + /* MapViewOfFile()ing zero bytes would map to the end of the file; match *nix behavior instead */ + if (range->length + delta == 0) { + return PHP_STREAM_OPTION_RETURN_ERR; + } + data->last_mapped_addr = MapViewOfFile(data->file_mapping, acc, 0, loffs, range->length + delta); if (data->last_mapped_addr) { diff --git a/main/streams/streams.c b/main/streams/streams.c index f1f8bf7eab6..cf411a1dd37 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -1573,29 +1573,48 @@ PHPAPI int _php_stream_copy_to_stream_ex(php_stream *src, php_stream *dest, size if (php_stream_mmap_possible(src)) { char *p; - size_t mapped; - p = php_stream_mmap_range(src, php_stream_tell(src), maxlen, PHP_STREAM_MAP_MODE_SHARED_READONLY, &mapped); + do { + size_t chunk_size = (maxlen == 0 || maxlen > PHP_STREAM_MMAP_MAX) ? PHP_STREAM_MMAP_MAX : maxlen; + size_t mapped; - if (p) { - ssize_t didwrite = php_stream_write(dest, p, mapped); - if (didwrite < 0) { - *len = 0; - return FAILURE; + p = php_stream_mmap_range(src, php_stream_tell(src), chunk_size, PHP_STREAM_MAP_MODE_SHARED_READONLY, &mapped); + + if (p) { + ssize_t didwrite; + + if (php_stream_seek(src, mapped, SEEK_CUR) != 0) { + php_stream_mmap_unmap(src); + break; + } + + didwrite = php_stream_write(dest, p, mapped); + if (didwrite < 0) { + *len = haveread; + return FAILURE; + } + + php_stream_mmap_unmap(src); + + *len = haveread += didwrite; + + /* we've got at least 1 byte to read + * less than 1 is an error + * AND read bytes match written */ + if (mapped == 0 || mapped != didwrite) { + return FAILURE; + } + if (mapped < chunk_size) { + return SUCCESS; + } + if (maxlen != 0) { + maxlen -= mapped; + if (maxlen == 0) { + return SUCCESS; + } + } } - - php_stream_mmap_unmap_ex(src, mapped); - - *len = didwrite; - - /* we've got at least 1 byte to read - * less than 1 is an error - * AND read bytes match written */ - if (mapped > 0 && mapped == didwrite) { - return SUCCESS; - } - return FAILURE; - } + } while (p); } while(1) {