-
-
Notifications
You must be signed in to change notification settings - Fork 677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
libc (malloc?) crash when returning false in true streaming custom target #4178
Comments
Hi @L3tum, I tried to make a reproducer: #!/usr/bin/env php
<?php
require dirname(__DIR__) . '/vendor/autoload.php';
use Jcupitt\Vips;
if (count($argv) != 4) {
echo "usage: $argv[0] IN-FILE OUT-FILE FORMAT\n";
echo " eg.: $argv[0] ~/pics/k2.jpg x.tif .tif[tile,pyramid]\n";
exit(1);
}
$in_file = fopen($argv[1], 'r');
$source = new Vips\SourceCustom();
$source->onRead(function ($bufferLength) use (&$in_file) {
// return 0 for EOF, -ve for read error
return fread($in_file, $bufferLength);
});
// seek is optional
$source->onSeek(function ($offset, $whence) use (&$in_file) {
if (fseek($in_file, $offset, $whence)) {
return -1;
}
return ftell($in_file);
});
// open for write and read ... formats like tiff need to be able to seek back
// in the output and update bytes later
$out_file = fopen($argv[2], 'w+');
$out_count = 0;
$target = new Vips\TargetCustom();
$target->onWrite(function ($buffer) use (&$out_file, &$out_count) {
$result = fwrite($out_file, $buffer);
$out_count += 1;
if ($out_count > 10) {
return -1;
}
if ($result === false) {
// IO error
return -1;
} else {
return $result;
}
});
// read and seek are optional
$target->onSeek(function ($offset, $whence) use (&$out_file) {
if (fseek($out_file, $offset, $whence)) {
return -1;
}
return ftell($out_file);
});
$target->onRead(function ($bufferLength) use (&$out_file) {
return fread($out_file, $bufferLength);
});
$image = Vips\Image::newFromSource($source);
for ($i = 0; $i < 10; $i++) {
echo "loop ", $i, "\n";
try {
$image->writeToTarget($target, $argv[3]);
} catch (Exception $e) {
echo "Caught exception: ", $e->getMessage(), "\n";
}
} But it seems to work fine, on ubuntu 24.04 at least. I see:
Though Could you make a small program I can run that shows the error? |
That code was adapted from |
Ah maybe that's all the problem is? You need to return -1 for an error, just like C's https://libvips.github.io/php-vips/classes/Jcupitt-Vips-TargetCustom.html#method_onWrite Since there's a |
Updated docs: libvips/php-vips@5f8ed10 |
Hahaha, you were faster than me. Thanks for all the work! Since I wrote the docs in php-vips myself I feel kind of stupid now. Can you check how it's handled in pyvips? I'm pretty sure I looked at the typings of that to figure out what should and shouldn't be expected in PHP, so maybe the docs there are ambiguous as well. |
I wonder if this is related to commit php/php-src@f547412, i.e. the
I think in some cases we also treat libvips/libvips/iofuncs/target.c Lines 467 to 470 in 937665b
(assuming |
Yes, you cannot throw across FFI in any language, I think. It has to be error codes. Maybe |
Yes, it's -1 in pyvips too. The -1 is a libvips thing, it's not part of the binding, so it'll be the same everywhere. |
Bug report
Describe the bug
This may just be me misusing the API, but I'm not sure how else do to what I need to do, and it should™ work from what I understand. When you write a custom target for streaming (in PHP) and return
false
from theonWrite
call (similar to whatfwrite
would do when the write fails), then you'll get an Exception from libvips that doesn't seem well handled in the first place (I'd guess an exception passed through from the underlying library, because it contains a different exception message depending on the target image format, for example). Shortly after receiving that exception, libc (sometimes™, around 80% of the time I'd guess) crashes with an error related to malloc (usuallydoubly-linked list corrupted
).I'm not at a capable PC right now, so it could be the case this is just PHP FFI and other languages handle it better (i.e. without crashing).
To Reproduce
Steps to reproduce the behavior:
false
at some pointdouble-linked list corrupted
which is apparently related to malloc. There was also a double-free error once. A coredump is generated as wellExpected behavior
Libvips should ideally handle the response stream stopping without crashing libc, and potentially without throwing an exception (since it's unexpected) or at least document the exception and handle the underlying exception by wrapping it in a more general vips exception (like "response stream broken" rather than the image format specific messages). It should also be documented how to gracefully stop the streaming ideally from the custom target itself (since that would be the location where, for example, a client going away would be handled).
Actual behavior
Image format specific message in exception coupled with a crashing libc
Screenshots
Here's the exception message for jpeg for example:
For PNG it's:
Environment
(please complete the following information)
Additional context
I couldn't identify all exception messages, since some of them just say
libvips error
and that's it. So I couldn't confirm if some image formats actually handle this case and libvips crashes, or if none of them handle it. I've also had a weird behaviour where the next call to libvips would return the result of the previous (completely unrelated) call for generating an image, but I haven't identified where that may be coming from, so is just meant as additional info that may help you. I will try to write a small reproducer, but I'm writing this bug report from my phone (cause no capable PC as I just moved).The text was updated successfully, but these errors were encountered: