kornelski / pngquant

Lossy PNG compressor — pngquant command based on libimagequant library
https://pngquant.org
Other
5.25k stars 487 forks source link

Memory leak #328

Open doublex opened 5 years ago

doublex commented 5 years ago

ASAN reports this memory leak:

Direct leak of 41408 byte(s) in 1 object(s) allocated from:
    #0 0x7d6830 in malloc
    #1 0xccd03c in liq_aligned_malloc pngquant/libimagequant.c:472
    #2 0xccd913 in liq_image_use_low_memory pngquant/libimagequant.c:563
    #3 0xccd913 in liq_image_create_internal pngquant/libimagequant.c:606
    #4 0x1088ce3 in read_image pngquant.c:602
    #5 0x1088ce3 in pngquant_file pngquant.c:644
    #6 0x108a7d8 in pngquant pngquant.c:709
kornelski commented 5 years ago

Does it happen with some specific image?

doublex commented 5 years ago

E.g. this file: leak

Code

pngquant( "/tmp/leak.png", "/tmp/leak-opt.png" );
doublex commented 5 years ago

If it helps I can provide more PNGs.

doublex commented 5 years ago

Testcase: testcase.c.gz

Steps to reproduce:

cd /tmp
wget https://github.com/kornelski/pngquant/files/3316726/testcase.c.gz
gzip -d testcase.c.gz
git clone https://github.com/ImageOptim/libimagequant.git
gcc -g -fsanitize=address -fno-omit-frame-pointer -o testcase testcase.c libimagequant/blur.c libimagequant/kmeans.c libimagequant/libimagequant.c libimagequant/mediancut.c libimagequant/mempool.c libimagequant/nearest.c libimagequant/pam.c -lm -lpng
wget https://user-images.githubusercontent.com/274624/55652572-25d31680-57ec-11e9-8323-ec038fb634ad.png
./testcase
doublex commented 5 years ago

This patch eliminates the memleak:

*** a/libimagequant.c   2019-06-22 20:46:26.599983591 +0200
--- b/libimagequant.c   2019-06-22 20:46:13.031177165 +0200
*************** LIQ_EXPORT LIQ_NONNULL liq_error liq_his
*** 560,565 ****
--- 560,567 ----

  LIQ_NONNULL static bool liq_image_use_low_memory(liq_image *img)
  {
+     if (img->temp_f_row)
+         img->free(img->temp_f_row);
      img->temp_f_row = img->malloc(sizeof(img->f_pixels[0]) * LIQ_TEMP_ROW_WIDTH(img->width) * omp_get_max_threads());
      return img->temp_f_row != NULL;
  }
doublex commented 5 years ago

@kornelski Maybe the fix is to avoid calling "liq_image_use_low_memory()" twice? Please do not miss "testcase.c.gz".

kornelski commented 5 years ago

I'm taking a break from coding. If you submit it as a pull request, I'll merge it.