Fix potential memory leaks in webpsave#4107
Conversation
jcupitt
left a comment
There was a problem hiding this comment.
How about adding a call to vips_foreign_save_webp_unset() to vips_foreign_save_webp_dispose()? Then we could get rid of all these other calls on the error paths, since the operation will always be disposed pretty rapidly during error handling.
We could leave a call at the end of _build(), since it can potentially free a useful amount of memory that won't otherwise get disposed for a long time.
|
Sounds good! I've simplified the cleanup handling with 1af0ed7 |
jcupitt
left a comment
There was a problem hiding this comment.
Ahhh much safer and prettier!
|
Perhaps this should target the |
a36c89a to
1af0ed7
Compare
|
Hmm strange, I wonder why the macOS test-suite step is failing now.. it passed in my workflow: https://github.com/dloebl/libvips/actions/runs/10540080027/job/29204347884
Maybe a retry helps |
|
I could reproduce that failure with libwebp 1.3.2 using this test script:
#!/usr/bin/env bash
set -ex
n=$1
exit_code=0
for (( i = 0; i < n; i++ )); do
echo "run $i"
vips webpsave_buffer test/test-suite/images/invalid_multiframe.gif[n=-1] || exit_code=$?
if [ $exit_code -ne 0 ]; then
echo "vips exited with non-zero exit code ($exit_code) after $i runs" >&2
exit $exit_code
fi
done$ ./check-regression.sh 1024
[...]
vips exited with non-zero exit code (1) after 63 runsCurious. |
|
Yeah, it's strange.. the test is definitely flaky. |
|
Maybe some sort of timing issue with threads? |
7b5413e to
2089e9e
Compare
Found while fuzzing locally with #4103:
To Reproduce
ToDo: