Fix memory leak in libjpeg-turbo decoder implementation in case of corrupted images #4138
Conversation
…rrupted images Signed-off-by: Joaquin Anton <janton@nvidia.com>
| return nullptr; | ||
| } | ||
|
|
||
| jpeg_create_decompress(&cinfo); | ||
| auto destroy_cinfo = AtScopeExit([&cinfo]() { | ||
| jpeg_destroy_decompress(&cinfo); |
There was a problem hiding this comment.
this is crucial. Some paths ended up without calling jpeg_destroy_decompress which was the source of the leak
| @@ -546,23 +524,6 @@ uint8* Uncompress(const void* srcdata, int datasize, | |||
| return dstdata; | |||
| } | |||
|
|
|||
| uint8* Uncompress(const void* srcdata, int datasize, | |||
| @@ -606,196 +567,5 @@ bool GetImageInfo(const void* srcdata, int datasize, int* width, int* height, | |||
| return true; | |||
| } | |||
|
|
|||
| // ----------------------------------------------------------------------------- | |||
| FewerArgsForCompiler argball(datasize, flags, nwarn, | ||
| std::move(allocate_output)); | ||
| const UncompressFlags& flags) { | ||
| FewerArgsForCompiler argball(datasize, flags); |
There was a problem hiding this comment.
I guess now we could get rid of that, too.
There was a problem hiding this comment.
It is used to keep some state too, apparently. So I'll leave it for now.
| FewerArgsForCompiler argball(datasize, flags, nwarn, | ||
| std::move(allocate_output)); | ||
| const UncompressFlags& flags) { | ||
| FewerArgsForCompiler argball(datasize, flags); |
There was a problem hiding this comment.
It is used to keep some state too, apparently. So I'll leave it for now.
|
!build |
|
CI MESSAGE: [5548261]: BUILD STARTED |
|
CI MESSAGE: [5548261]: BUILD FAILED |
Signed-off-by: Joaquin Anton <janton@nvidia.com>
|
!build |
|
CI MESSAGE: [5550334]: BUILD STARTED |
|
CI MESSAGE: [5550334]: BUILD PASSED |
| FewerArgsForCompiler argball(datasize, flags, nwarn, | ||
| std::move(allocate_output)); | ||
| const UncompressFlags& flags) { | ||
| FewerArgsForCompiler argball(datasize, flags); | ||
| uint8* const dstdata = UncompressLow(srcdata, &argball); |
There was a problem hiding this comment.
So here we releasded the ownership of the memory and the caller of the function must take care of releasing it, right? As a food for thought, maybe this function should also return smart pointer? It looks a bit brittle.
Signed-off-by: Joaquin Anton <janton@nvidia.com>
|
!build |
|
CI MESSAGE: [5557749]: BUILD STARTED |
|
CI MESSAGE: [5557749]: BUILD FAILED |
|
CI MESSAGE: [5557749]: BUILD PASSED |
…rrupted images (NVIDIA#4138) Signed-off-by: Joaquin Anton <janton@nvidia.com>
…rrupted images (#4138) Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton janton@nvidia.com
Category:
Bug fix
Description:
.release()on the successful path.Additional information:
Affected modules and functionalities:
fn.decoders.imageKey points relevant for the review:
Replacement of manual memory handling with unique_ptr
Tests:
Checked memory consumption locally with the dataset provided in the mentioned issue
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: DALI-2944