Beep6581 / RawTherapee

A powerful cross-platform raw photo processing program
https://rawtherapee.com
GNU General Public License v3.0
2.8k stars 316 forks source link

Wrong delete statement and memory leak #2186

Closed Beep6581 closed 9 years ago

Beep6581 commented 9 years ago

Originally reported on Google Code with ID 2202

Valgrind found one wrong delete statement and a lot of memory leaks.
This patch fixes the wrong delete statement and one big memory leak, which occurs each
time an image is added to or removed from the queue. Leak size depends on thumb size
(e.g. with ThumbnailSizeQueue=160 it was 178800 byte per image). It also leaked when
starting RT with a non empty queue.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-01-15 12:28:53


Beep6581 commented 9 years ago
I ran RT with all 45 raw /test_images/ using Default.pp3.
First three show unpatched RT, second three the patched RT.
The gap is where RT crashed, strangely enough it crashed in the same place each time.
salon.nef was the next one in the queue. I reran it and resumed the queue.
http://imgur.com/a/LOAGl#0

Reported by entertheyoni on 2014-01-15 14:34:39

Beep6581 commented 9 years ago
I think, running the queue is not much involved in this case. It's the add or remove
from images to or from the queue (every time a thumb in the queue changes)
I tried with 20 images in the queue, started RT, increased and decreased the thumb
size in the queue. This lead to a leak of about 5 MB each time I clicked on + or -
in the queue tab. 
You can also test with adding and removing images to (from) the queue several times.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-01-15 14:52:17

Beep6581 commented 9 years ago
Oh, so I checked for the wrong thing.

FWIW, here's the rest:
I added /test_images/ to the queue 10 times and RT always crashes after ryanair.pef
(which gets developed fine) with salon.nef being the first in the queue after restart.
When I restart, the first 7 images in the queue have no thumbnail, as in the screenshot.
Same every time.
http://imgur.com/a/SyFm4#0

I'll investiage the crash in a moment, need to take the doggie for a walk first.

Reported by entertheyoni on 2014-01-15 15:22:50

Beep6581 commented 9 years ago
About the crash: I did the same test as you. After some 150 images or so RT didn't crash
but freeze. In my case after restarting RT, the first five images had no thumbs for
some seconds, but then they came back miraculously. The image where it crashed wasn't
the same as in your test. Now testing without patch...

Reported by heckflosse@i-weyrich.de on 2014-01-15 16:09:14

Beep6581 commented 9 years ago
I reproduced it without your patch so it's probably unrelated. I created issue 2187
so as not to hijack this.

The patch seems to work. The left half is unpatched RT with me enlarging and shrinking
the size of queue thumbs, deleting them all and re-adding them a few times. The right
half shows patched RT with me doing the same.
http://i.imgur.com/RQMqVCX.png

Reported by entertheyoni on 2014-01-15 17:17:58

Beep6581 commented 9 years ago
Great. Thanks for you test.

Reported by heckflosse@i-weyrich.de on 2014-01-15 17:26:44

Beep6581 commented 9 years ago
Would it make sense to provide the windows build after this patch is committed?

Reported by michaelezra000 on 2014-01-15 18:31:13

Beep6581 commented 9 years ago
I agree!

Reported by heckflosse@i-weyrich.de on 2014-01-15 18:42:25

Beep6581 commented 9 years ago
Committed to default.

Reported by heckflosse@i-weyrich.de on 2014-01-15 22:15:37

Beep6581 commented 9 years ago

Reported by heckflosse@i-weyrich.de on 2014-01-19 14:53:38