Fix memory leaks in ext-tidy #10545
Fix memory leaks in ext-tidy #10545
Conversation
| RETURN_THROWS(); | ||
| } | ||
|
|
||
| tidy_instanciate(tidy_ce_doc, return_value); |
There was a problem hiding this comment.
looks good but are you fixing just a bit more than memory leak here ?
There was a problem hiding this comment.
never mind, just saw the rest of your commit message.
| const MIN_FILE_SIZE = 4_294_967_295; | ||
| $total_bytes = 0; | ||
| $s = str_repeat("a", 10_000); | ||
| while ($total_bytes < MIN_FILE_SIZE) { | ||
| $bytes_written = fwrite($file, $s); | ||
| if ($bytes_written === false) { | ||
| echo "Didn't write bytes\n"; | ||
| } | ||
| $total_bytes += $bytes_written; | ||
| } |
There was a problem hiding this comment.
Is a sparse file sufficient to reproduce the issue? You should be able to create one by seeking to a large offset and then writing something. This should be much faster and take up less disk storage when running the test.
There was a problem hiding this comment.
I didn't know about sparse files until now. Let me try that.
There was a problem hiding this comment.
The test LGTM now. I trust you tested that it still does what you expect it to do.
We must not instantiate the object prior checking error conditions Moreover, we need to release the HUGE amount of memory for files which are over 4GB when throwing a ValueError
Girgias
force-pushed
the
tidy-memory-leak-fixes
branch
from
19c1e20 to
ccc21ec
Compare
Girgias
force-pushed
the
tidy-memory-leak-fixes
branch
from
ca67517 to
54b5d61
Compare
| --SKIPIF-- | ||
| <?php | ||
| if (PHP_INT_SIZE != 8) die("skip this test is for 64bit platform only"); | ||
| if (getenv("SKIP_SLOW_TESTS")) die("skip slow test"); |
There was a problem hiding this comment.
Do we really need this test? It causes issues in CI. We should at least disable it for ASAN/UBSAN/MSAN.
There was a problem hiding this comment.
I'd prefer to have such a test, but skiping it for ASAN/UBSAN/MSAN can make sense, as this what caught by ZMM anyway.
4 participants
We must not instantiate the object prior to checking error conditions.
Moreover, we need to release the HUGE amount of memory for files which are over 4GB when throwing a ValueError.