Skip to content
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

Fix memory leak in deflate decoder #37

Closed
wants to merge 4 commits into from

Conversation

gigony
Copy link
Contributor

@gigony gigony commented May 19, 2021

I did a silly mistake in deflate decoder which caused a memory leak when loading the Deflate-compressed TIFF image.

This patch will free allocated memory in decode_deflate() method.

For the next steps to prevent this kind of errors:

  • Make use of a safe pointer (smart pointer) or scoped deleter for 3rdparty library/internal memory allocation.
  • Run unit/integration tests with memory sanitizer/tracker (e.g., valgrind/asan)

@gigony gigony added breaking Introduces a breaking change bug Something isn't working labels May 19, 2021
@gigony gigony requested a review from a team as a code owner May 19, 2021 00:43
@quasiben
Copy link
Member

rerun tests

@gigony
Copy link
Contributor Author

gigony commented May 20, 2021

Build aborted due to timeout:

https://gpuci.gpuopenanalytics.com/job/rapidsai/job/gpuci-v0.20/job/cucim/job/prb/job/cucim-gpu-test/CUDA=11.0,GPU_LABEL=gpu,OS=ubuntu16.04,PYTHON=3.7/55/console

Examining conflict for rapids-build-env libarchive libxml2 icu:  77%|███████▋  | 71/92 [20:48<05:27, 15.62s/it]
Examining conflict for rapids-build-env cucim:  77%|███████▋  | 71/92 [21:08<05:27, 15.62s/it]                 
Examining conflict for rapids-build-env cucim:  78%|███████▊  | 72/92 [21:08<05:39, 16.95s/it]
Build timed out (after 40 minutes). Marking the build as failed.

@jakirkham
Copy link
Member

Yeah this came up on Slack as well. Wondering if this is just due to the fact that we haven't fully moved to the new RAPIDS version scheme. If so, then it might be worth focusing on getting PR ( #41 ) in first and working with AJ to get cuCIM updated to the new version scheme

@gigony gigony requested a review from a team as a code owner June 5, 2021 07:37
@gigony gigony added non-breaking Introduces a non-breaking change and removed breaking Introduces a breaking change labels Jun 5, 2021
@gigony
Copy link
Contributor Author

gigony commented Jun 8, 2021

this change merged by other PR.

#close

@gigony gigony closed this Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants