Valgrind "definitely lost" backtrace

[Richard Kimberly Heck]

On 2/17/20 2:08 PM, Scott Kostyshak wrote:
> On Mon, Feb 17, 2020 at 12:35:19PM -0500, Richard Kimberly Heck wrote:
>> On 2/17/20 8:17 AM, Pavel Sanda wrote:
>>> On Sun, Feb 16, 2020 at 07:21:17PM -0500, Scott Kostyshak wrote:
>>>> On Sat, Feb 15, 2020 at 09:42:52PM +0100, Pavel Sanda wrote:
>>>>> On Fri, Feb 14, 2020 at 08:09:09PM -0500, Scott Kostyshak wrote:
>>>>>> Thanks to both of you for taking a look. I was hoping to investigate
>>>>>> this memory leak but it seems to be beyond my knowledge.
>>>>> Do you have recipy to reproduce valgrind's warning?
>>>> 1. I run the following command (you will need to adapt it):
>>>>
>>>>   valgrind --leak-check=full --track-origins=yes --log-file=valgrind.log ~/lyxbuilds/master/CMakeBuild/bin/lyx -userdir ~/lyxbuilds/master/user-dir
>>>>
>>>> 2. Do "ctrl + n" to start a new document.
>>>>
>>>> 3. Type "abc".
>>>>
>>>> 4. Quit LyX and respond "no" when prompted whether to save the document.
>>>>
>>>> I attach the full log in case it is useful. The other warnings involve
>>>> external libraries.
>>>>
>>>> Can anyone else reproduce?
>>> I can reproduce. I looked now more carefully in the code, but couldn't
>>> find where in the code are delete operators corresponding to the new operator
>>> at TocModel.cpp:362.
>>>
>>> So this looks like we just forget to free memory at the end of TocModels
>>> life. As I know little about ToCXXX routines I would appreciate someone (Riki?)
>>> knowing the context ack my change before comitting.
>> I don't see how that could hurt. And it looks to me like the TocModels
>> objects are per-buffer. If so, then closing a file would leak, and I
>> think the patch would prevent that leak. I take it that it does silence
>> the warning?
> I'm wondering if I'm interpreting the above paragraph correctly. Here's
> my interpretation: if the TocModels were per-LyX session, then although
> it is still technically a memory leak, it would not have any benefit to
> fix it because LyX is being exited anyway so all of the memory LyX has
> will be given back to the OS. That is, the memory leak has a short life.
> On the other hand, since TocModels objects are per-buffer, these memory
> leaks could have longer lives because users can close buffers without
> exiting LyX. Did I understand the intuition correctly?

Yes, precisely. And I think they are per-buffer. It may well be that
valgrind is complaining (on your script for reproducing) when the Buffer
and its associated TocModel are destroyed, which it is during the exit
process, at which point the memory is leaked---though not for very long,
in that case. But in other cases.... In any event, I think it is worth
fixing even if I'm wrong, if only because someone could come along and
make the TocModels per-buffer, and then we have a real leak.

Does Pavel's patch help?

Riki