Skip to content

Commit 5e9e9c9

Browse files
committed
Fix GH-13071: Copying large files using mmap-able source streams may exhaust available memory and fail
Commit 5cbe5a5 disabled chunking for all writes to streams. However, user streams have a callback where code is executed on data that is subject to the memory limit. Therefore, when using large writes or stream_copy_to_stream/copy the memory limit can easily be hit with large enough data. To solve this, we reintroduce chunking for userspace streams. Users have control over the chunk size, which is neat because they can improve the performance by setting the chunk size if that turns out to be a bottleneck. In an ideal world, we add an option so we can "ask" the stream whether it "prefers" chunked writes, similar to how we have php_stream_mmap_supported & friends. However, that cannot be done on stable branches. Closes GH-13136.
1 parent b33e3eb commit 5e9e9c9

File tree

6 files changed

+80
-8
lines changed

6 files changed

+80
-8
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ PHP NEWS
3030
. Fixed bug GH-13138 (Randomizer::pickArrayKeys() does not detect broken
3131
engines). (timwolla)
3232

33+
- Streams:
34+
. Fixed bug GH-13071 (Copying large files using mmap-able source streams may
35+
exhaust available memory and fail). (nielsdos)
36+
3337
18 Jan 2024, PHP 8.2.15
3438

3539
- Core:

ext/standard/tests/file/gh13136.phpt

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
--TEST--
2+
GH-13071 (Copying large files using mmap-able source streams may exhaust available memory and fail)
3+
--FILE--
4+
<?php
5+
6+
class CustomStream {
7+
public $context;
8+
protected $file;
9+
protected $seekable;
10+
public static int $writes = 0;
11+
12+
public function stream_open($path, $mode, $options, &$opened_path) {
13+
$path = $this->trim_path($path);
14+
$this->file = fopen($path, $mode);
15+
return true;
16+
}
17+
18+
public function stream_close() {
19+
fclose($this->file);
20+
return true;
21+
}
22+
23+
public function stream_write($data) {
24+
self::$writes++;
25+
return fwrite($this->file, $data);
26+
}
27+
28+
public function url_stat($path, $flags) {
29+
return false;
30+
}
31+
32+
private function trim_path(string $path): string {
33+
return substr($path, strlen("up://"));
34+
}
35+
}
36+
37+
file_put_contents(__DIR__ . "/gh13071.tmp", str_repeat("a", 1024 * 1024 * 8));
38+
39+
stream_wrapper_register("up", CustomStream::class, STREAM_IS_URL);
40+
41+
$old_limit = ini_get("memory_limit");
42+
ini_set("memory_limit", memory_get_usage(true) + 5 * 1024 * 1024);
43+
copy(__DIR__ . "/gh13071.tmp", "up://" . __DIR__ . "/gh13071.out.tmp");
44+
ini_set("memory_limit", $old_limit);
45+
46+
echo "Done ", CustomStream::$writes, " writes\n";
47+
48+
?>
49+
--CLEAN--
50+
<?php
51+
@unlink(__DIR__ . "/gh13071.tmp");
52+
@unlink(__DIR__ . "/gh13071.out.tmp");
53+
?>
54+
--EXPECT--
55+
Done 1024 writes

ext/standard/tests/file/userstreams_006.phpt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,6 @@ bool(true)
3434
option: 3, 2, 50
3535
int(-1)
3636
int(8192)
37-
size: 70
37+
size: 42
38+
size: 28
3839
int(70)

ext/standard/tests/streams/set_file_buffer.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,5 @@ option: %d, %d, %d
3939
int(%i)
4040
int(%d)
4141
size: %d
42+
size: 28
4243
int(%d)

ext/standard/tests/streams/stream_set_chunk_size.phpt

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ echo "should return previous chunk size (8192)\n";
3535
var_dump(stream_set_chunk_size($f, 1));
3636
echo "should be read without buffer (\$count == 10000)\n";
3737
var_dump(strlen(fread($f, 10000)));
38-
echo "should have no effect on writes\n";
38+
echo "should elicit 3 writes\n";
3939
var_dump(fwrite($f, str_repeat('b', 3)));
4040

4141
echo "should return previous chunk size (1)\n";
@@ -46,7 +46,7 @@ echo "should elicit one read of size 100 (chunk size)\n";
4646
var_dump(strlen(fread($f, 50)));
4747
echo "should elicit no read because there is sufficient cached data\n";
4848
var_dump(strlen(fread($f, 50)));
49-
echo "should have no effect on writes\n";
49+
echo "should elicit 3 writes\n";
5050
var_dump(strlen(fwrite($f, str_repeat('b', 250))));
5151

5252
echo "\nerror conditions\n";
@@ -68,8 +68,10 @@ int(8192)
6868
should be read without buffer ($count == 10000)
6969
read with size: 10000
7070
int(10000)
71-
should have no effect on writes
72-
write with size: 3
71+
should elicit 3 writes
72+
write with size: 1
73+
write with size: 1
74+
write with size: 1
7375
int(3)
7476
should return previous chunk size (1)
7577
int(1)
@@ -81,8 +83,10 @@ read with size: 100
8183
int(50)
8284
should elicit no read because there is sufficient cached data
8385
int(50)
84-
should have no effect on writes
85-
write with size: 250
86+
should elicit 3 writes
87+
write with size: 100
88+
write with size: 100
89+
write with size: 50
8690
int(3)
8791

8892
error conditions

main/streams/streams.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1133,8 +1133,15 @@ static ssize_t _php_stream_write_buffer(php_stream *stream, const char *buf, siz
11331133
stream->ops->seek(stream, stream->position, SEEK_SET, &stream->position);
11341134
}
11351135

1136+
/* See GH-13071: userspace stream is subject to the memory limit. */
1137+
size_t chunk_size = count;
1138+
if (php_stream_is(stream, PHP_STREAM_IS_USERSPACE)) {
1139+
/* If the stream is unbuffered, we can only write one byte at a time. */
1140+
chunk_size = stream->chunk_size;
1141+
}
1142+
11361143
while (count > 0) {
1137-
ssize_t justwrote = stream->ops->write(stream, buf, count);
1144+
ssize_t justwrote = stream->ops->write(stream, buf, MIN(chunk_size, count));
11381145
if (justwrote <= 0) {
11391146
/* If we already successfully wrote some bytes and a write error occurred
11401147
* later, report the successfully written bytes. */

0 commit comments

Comments
 (0)