Re: Memory leak from list
On Sun, Feb 23, 2020 at 05:00:43PM -0500, Richard Kimberly Heck wrote:
> On 2/23/20 4:11 PM, Scott Kostyshak wrote:
> > On Sun, Feb 23, 2020 at 03:54:06PM -0500, Richard Kimberly Heck wrote:
> >> On 2/23/20 2:31 PM, Scott Kostyshak wrote:
> >>> On Sun, Feb 23, 2020 at 12:50:42PM -0500, Richard Kimberly Heck wrote:
> On 2/23/20 8:23 AM, Scott Kostyshak wrote:
> > On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote:
> >> On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote:
> >>> On 2/18/20 6:07 PM, Scott Kostyshak wrote:
> Valgrind gave me the following error:
>
> ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are
> definitely lost in loss record 5,165 of 5,862
> ==732==at 0x483AE63: operator new(unsigned long) (in
> /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const
> (Buffer.cpp:661)
> ==732==by 0x11E583C: lyx::(anonymous
> namespace)::copyToTempBuffer(lyx::ParagraphList const&,
> std::shared_ptr) (CutAndPaste.cpp:582)
> ==732==by 0x11E5C6A: lyx::(anonymous
> namespace)::putClipboard(lyx::ParagraphList const&,
> std::shared_ptr,
> std::__cxx11::basic_string,
> std::allocator > const&, lyx::BufferParams)
> (CutAndPaste.cpp:613)
> ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor
> const&, std::__cxx11::basic_string std::char_traits, std::allocator > const&)
> (CutAndPaste.cpp:1123)
> ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor
> const&) (CutAndPaste.cpp:1024)
> ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&,
> lyx::FuncRequest&) (Text3.cpp:1593)
> ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&,
> lyx::FuncRequest&) (InsetText.cpp:339)
> ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&,
> lyx::FuncRequest&) (Inset.cpp:325)
> ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest
> const&) (Cursor.cpp:825)
> ==732==by 0x1816F4F:
> lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest
> const&, lyx::DispatchResult&) (GuiView.cpp:3878)
> ==732==by 0x181B959:
> lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&,
> lyx::DispatchResult&) (GuiView.cpp:4569)
>
> It comes from the following line (Buffer.cpp:661):
>
> cloned_buffers.push_back(new CloneList);
>
> Currently cloned_buffers is a list. Would it make sense
> to make it a list of *smart* pointers instead? Alternatively we
> could make a class and then make a custom destructor that would free
> the CloneLists that the list elements point to?
> >>> This is some kind of thinko, probably on my part. The code at line 549
> >>> was supposed to be cleaning this up, but it actually only removes the
> >>> entry from the list.
> >>>
> >>> If it works to make it a smart pointer of some kind, then that would
> >>> be
> >>> simplest. But I think we could just do something like:
> >>>
> >>> else {
> >>> delete(*it);
> >>> cloned_buffers.erase(it);
> >>> }
> >> Ah that makes sense.
> > Riki, I propose that you commit. Thanks for the fix.
> Just to check: You've verified this fixes the problem?
> >>> I just tried to reproduce the original error (without the fix) and could
> >>> not. I originally got the message from copying something and compiling.
> >>> I just tried doing that with the Customization and Embedded Objects
> >>> manuals, but I could not trigger the error.
> >> Committed, then.
> > Thanks for the fix. I'll continue testing LyX with Valgrind at some
> > point in the future. Right now I'm tired of the waiting. I might look
> > into the suggestions that Neven gave.
>
> Actually, that is all wrong, and now I'm very confused. The patch causes
> a double delete exception here, so I have reverted it. The line
>
> delete d->clone_list_;
>
> already does this deletion, since *it just is d->clone_list_.
>
> I've committed another patch with a shared_ptr. That has at least to help.
OK thanks for your work on this. I'll keep an eye out when testing with
Valgrind to see if something related comes up.
Scott
signature.asc
Description: PGP signature
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On 2/23/20 4:11 PM, Scott Kostyshak wrote:
> On Sun, Feb 23, 2020 at 03:54:06PM -0500, Richard Kimberly Heck wrote:
>> On 2/23/20 2:31 PM, Scott Kostyshak wrote:
>>> On Sun, Feb 23, 2020 at 12:50:42PM -0500, Richard Kimberly Heck wrote:
On 2/23/20 8:23 AM, Scott Kostyshak wrote:
> On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote:
>> On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote:
>>> On 2/18/20 6:07 PM, Scott Kostyshak wrote:
Valgrind gave me the following error:
==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are
definitely lost in loss record 5,165 of 5,862
==732==at 0x483AE63: operator new(unsigned long) (in
/usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const
(Buffer.cpp:661)
==732==by 0x11E583C: lyx::(anonymous
namespace)::copyToTempBuffer(lyx::ParagraphList const&,
std::shared_ptr) (CutAndPaste.cpp:582)
==732==by 0x11E5C6A: lyx::(anonymous
namespace)::putClipboard(lyx::ParagraphList const&,
std::shared_ptr,
std::__cxx11::basic_string,
std::allocator > const&, lyx::BufferParams)
(CutAndPaste.cpp:613)
==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&,
std::__cxx11::basic_string,
std::allocator > const&) (CutAndPaste.cpp:1123)
==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&)
(CutAndPaste.cpp:1024)
==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&,
lyx::FuncRequest&) (Text3.cpp:1593)
==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&,
lyx::FuncRequest&) (InsetText.cpp:339)
==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&,
lyx::FuncRequest&) (Inset.cpp:325)
==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest
const&) (Cursor.cpp:825)
==732==by 0x1816F4F:
lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&,
lyx::DispatchResult&) (GuiView.cpp:3878)
==732==by 0x181B959:
lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&,
lyx::DispatchResult&) (GuiView.cpp:4569)
It comes from the following line (Buffer.cpp:661):
cloned_buffers.push_back(new CloneList);
Currently cloned_buffers is a list. Would it make sense
to make it a list of *smart* pointers instead? Alternatively we could
make a class and then make a custom destructor that would free the
CloneLists that the list elements point to?
>>> This is some kind of thinko, probably on my part. The code at line 549
>>> was supposed to be cleaning this up, but it actually only removes the
>>> entry from the list.
>>>
>>> If it works to make it a smart pointer of some kind, then that would be
>>> simplest. But I think we could just do something like:
>>>
>>> else {
>>> delete(*it);
>>> cloned_buffers.erase(it);
>>> }
>> Ah that makes sense.
> Riki, I propose that you commit. Thanks for the fix.
Just to check: You've verified this fixes the problem?
>>> I just tried to reproduce the original error (without the fix) and could
>>> not. I originally got the message from copying something and compiling.
>>> I just tried doing that with the Customization and Embedded Objects
>>> manuals, but I could not trigger the error.
>> Committed, then.
> Thanks for the fix. I'll continue testing LyX with Valgrind at some
> point in the future. Right now I'm tired of the waiting. I might look
> into the suggestions that Neven gave.
Actually, that is all wrong, and now I'm very confused. The patch causes
a double delete exception here, so I have reverted it. The line
delete d->clone_list_;
already does this deletion, since *it just is d->clone_list_.
I've committed another patch with a shared_ptr. That has at least to help.
Riki
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On Sun, Feb 23, 2020 at 03:54:06PM -0500, Richard Kimberly Heck wrote:
> On 2/23/20 2:31 PM, Scott Kostyshak wrote:
> > On Sun, Feb 23, 2020 at 12:50:42PM -0500, Richard Kimberly Heck wrote:
> >> On 2/23/20 8:23 AM, Scott Kostyshak wrote:
> >>> On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote:
> On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote:
> > On 2/18/20 6:07 PM, Scott Kostyshak wrote:
> >> Valgrind gave me the following error:
> >>
> >> ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are
> >> definitely lost in loss record 5,165 of 5,862
> >> ==732==at 0x483AE63: operator new(unsigned long) (in
> >> /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> >> ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const
> >> (Buffer.cpp:661)
> >> ==732==by 0x11E583C: lyx::(anonymous
> >> namespace)::copyToTempBuffer(lyx::ParagraphList const&,
> >> std::shared_ptr) (CutAndPaste.cpp:582)
> >> ==732==by 0x11E5C6A: lyx::(anonymous
> >> namespace)::putClipboard(lyx::ParagraphList const&,
> >> std::shared_ptr,
> >> std::__cxx11::basic_string,
> >> std::allocator > const&, lyx::BufferParams)
> >> (CutAndPaste.cpp:613)
> >> ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&,
> >> std::__cxx11::basic_string,
> >> std::allocator > const&) (CutAndPaste.cpp:1123)
> >> ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&)
> >> (CutAndPaste.cpp:1024)
> >> ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&,
> >> lyx::FuncRequest&) (Text3.cpp:1593)
> >> ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&,
> >> lyx::FuncRequest&) (InsetText.cpp:339)
> >> ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&,
> >> lyx::FuncRequest&) (Inset.cpp:325)
> >> ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest
> >> const&) (Cursor.cpp:825)
> >> ==732==by 0x1816F4F:
> >> lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&,
> >> lyx::DispatchResult&) (GuiView.cpp:3878)
> >> ==732==by 0x181B959:
> >> lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&,
> >> lyx::DispatchResult&) (GuiView.cpp:4569)
> >>
> >> It comes from the following line (Buffer.cpp:661):
> >>
> >> cloned_buffers.push_back(new CloneList);
> >>
> >> Currently cloned_buffers is a list. Would it make sense
> >> to make it a list of *smart* pointers instead? Alternatively we could
> >> make a class and then make a custom destructor that would free the
> >> CloneLists that the list elements point to?
> > This is some kind of thinko, probably on my part. The code at line 549
> > was supposed to be cleaning this up, but it actually only removes the
> > entry from the list.
> >
> > If it works to make it a smart pointer of some kind, then that would be
> > simplest. But I think we could just do something like:
> >
> > else {
> > delete(*it);
> > cloned_buffers.erase(it);
> > }
> Ah that makes sense.
> >>> Riki, I propose that you commit. Thanks for the fix.
> >> Just to check: You've verified this fixes the problem?
> > I just tried to reproduce the original error (without the fix) and could
> > not. I originally got the message from copying something and compiling.
> > I just tried doing that with the Customization and Embedded Objects
> > manuals, but I could not trigger the error.
>
> Committed, then.
Thanks for the fix. I'll continue testing LyX with Valgrind at some
point in the future. Right now I'm tired of the waiting. I might look
into the suggestions that Neven gave.
Scott
signature.asc
Description: PGP signature
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On 2/23/20 2:31 PM, Scott Kostyshak wrote:
> On Sun, Feb 23, 2020 at 12:50:42PM -0500, Richard Kimberly Heck wrote:
>> On 2/23/20 8:23 AM, Scott Kostyshak wrote:
>>> On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote:
On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote:
> On 2/18/20 6:07 PM, Scott Kostyshak wrote:
>> Valgrind gave me the following error:
>>
>> ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely
>> lost in loss record 5,165 of 5,862
>> ==732==at 0x483AE63: operator new(unsigned long) (in
>> /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const
>> (Buffer.cpp:661)
>> ==732==by 0x11E583C: lyx::(anonymous
>> namespace)::copyToTempBuffer(lyx::ParagraphList const&,
>> std::shared_ptr) (CutAndPaste.cpp:582)
>> ==732==by 0x11E5C6A: lyx::(anonymous
>> namespace)::putClipboard(lyx::ParagraphList const&,
>> std::shared_ptr,
>> std::__cxx11::basic_string,
>> std::allocator > const&, lyx::BufferParams)
>> (CutAndPaste.cpp:613)
>> ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&,
>> std::__cxx11::basic_string,
>> std::allocator > const&) (CutAndPaste.cpp:1123)
>> ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&)
>> (CutAndPaste.cpp:1024)
>> ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&,
>> lyx::FuncRequest&) (Text3.cpp:1593)
>> ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&,
>> lyx::FuncRequest&) (InsetText.cpp:339)
>> ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&,
>> lyx::FuncRequest&) (Inset.cpp:325)
>> ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest
>> const&) (Cursor.cpp:825)
>> ==732==by 0x1816F4F:
>> lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&,
>> lyx::DispatchResult&) (GuiView.cpp:3878)
>> ==732==by 0x181B959:
>> lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&,
>> lyx::DispatchResult&) (GuiView.cpp:4569)
>>
>> It comes from the following line (Buffer.cpp:661):
>>
>> cloned_buffers.push_back(new CloneList);
>>
>> Currently cloned_buffers is a list. Would it make sense to
>> make it a list of *smart* pointers instead? Alternatively we could make
>> a class and then make a custom destructor that would free the CloneLists
>> that the list elements point to?
> This is some kind of thinko, probably on my part. The code at line 549
> was supposed to be cleaning this up, but it actually only removes the
> entry from the list.
>
> If it works to make it a smart pointer of some kind, then that would be
> simplest. But I think we could just do something like:
>
> else {
> delete(*it);
> cloned_buffers.erase(it);
> }
Ah that makes sense.
>>> Riki, I propose that you commit. Thanks for the fix.
>> Just to check: You've verified this fixes the problem?
> I just tried to reproduce the original error (without the fix) and could
> not. I originally got the message from copying something and compiling.
> I just tried doing that with the Customization and Embedded Objects
> manuals, but I could not trigger the error.
Committed, then.
Riki
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On Sun, Feb 23, 2020 at 12:50:42PM -0500, Richard Kimberly Heck wrote:
> On 2/23/20 8:23 AM, Scott Kostyshak wrote:
> > On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote:
> >> On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote:
> >>> On 2/18/20 6:07 PM, Scott Kostyshak wrote:
> Valgrind gave me the following error:
>
> ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely
> lost in loss record 5,165 of 5,862
> ==732==at 0x483AE63: operator new(unsigned long) (in
> /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const
> (Buffer.cpp:661)
> ==732==by 0x11E583C: lyx::(anonymous
> namespace)::copyToTempBuffer(lyx::ParagraphList const&,
> std::shared_ptr) (CutAndPaste.cpp:582)
> ==732==by 0x11E5C6A: lyx::(anonymous
> namespace)::putClipboard(lyx::ParagraphList const&,
> std::shared_ptr,
> std::__cxx11::basic_string,
> std::allocator > const&, lyx::BufferParams)
> (CutAndPaste.cpp:613)
> ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&,
> std::__cxx11::basic_string,
> std::allocator > const&) (CutAndPaste.cpp:1123)
> ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&)
> (CutAndPaste.cpp:1024)
> ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&,
> lyx::FuncRequest&) (Text3.cpp:1593)
> ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&,
> lyx::FuncRequest&) (InsetText.cpp:339)
> ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&,
> lyx::FuncRequest&) (Inset.cpp:325)
> ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest
> const&) (Cursor.cpp:825)
> ==732==by 0x1816F4F:
> lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&,
> lyx::DispatchResult&) (GuiView.cpp:3878)
> ==732==by 0x181B959:
> lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&,
> lyx::DispatchResult&) (GuiView.cpp:4569)
>
> It comes from the following line (Buffer.cpp:661):
>
> cloned_buffers.push_back(new CloneList);
>
> Currently cloned_buffers is a list. Would it make sense to
> make it a list of *smart* pointers instead? Alternatively we could make
> a class and then make a custom destructor that would free the CloneLists
> that the list elements point to?
> >>> This is some kind of thinko, probably on my part. The code at line 549
> >>> was supposed to be cleaning this up, but it actually only removes the
> >>> entry from the list.
> >>>
> >>> If it works to make it a smart pointer of some kind, then that would be
> >>> simplest. But I think we could just do something like:
> >>>
> >>> else {
> >>> delete(*it);
> >>> cloned_buffers.erase(it);
> >>> }
> >> Ah that makes sense.
> > Riki, I propose that you commit. Thanks for the fix.
>
> Just to check: You've verified this fixes the problem?
I just tried to reproduce the original error (without the fix) and could
not. I originally got the message from copying something and compiling.
I just tried doing that with the Customization and Embedded Objects
manuals, but I could not trigger the error.
Scott
signature.asc
Description: PGP signature
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On 2/23/20 8:23 AM, Scott Kostyshak wrote:
> On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote:
>> On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote:
>>> On 2/18/20 6:07 PM, Scott Kostyshak wrote:
Valgrind gave me the following error:
==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely
lost in loss record 5,165 of 5,862
==732==at 0x483AE63: operator new(unsigned long) (in
/usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const
(Buffer.cpp:661)
==732==by 0x11E583C: lyx::(anonymous
namespace)::copyToTempBuffer(lyx::ParagraphList const&,
std::shared_ptr) (CutAndPaste.cpp:582)
==732==by 0x11E5C6A: lyx::(anonymous
namespace)::putClipboard(lyx::ParagraphList const&,
std::shared_ptr,
std::__cxx11::basic_string,
std::allocator > const&, lyx::BufferParams) (CutAndPaste.cpp:613)
==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&,
std::__cxx11::basic_string,
std::allocator > const&) (CutAndPaste.cpp:1123)
==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&)
(CutAndPaste.cpp:1024)
==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&,
lyx::FuncRequest&) (Text3.cpp:1593)
==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&,
lyx::FuncRequest&) (InsetText.cpp:339)
==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&,
lyx::FuncRequest&) (Inset.cpp:325)
==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest const&)
(Cursor.cpp:825)
==732==by 0x1816F4F:
lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&,
lyx::DispatchResult&) (GuiView.cpp:3878)
==732==by 0x181B959:
lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&,
lyx::DispatchResult&) (GuiView.cpp:4569)
It comes from the following line (Buffer.cpp:661):
cloned_buffers.push_back(new CloneList);
Currently cloned_buffers is a list. Would it make sense to
make it a list of *smart* pointers instead? Alternatively we could make a
class and then make a custom destructor that would free the CloneLists
that the list elements point to?
>>> This is some kind of thinko, probably on my part. The code at line 549
>>> was supposed to be cleaning this up, but it actually only removes the
>>> entry from the list.
>>>
>>> If it works to make it a smart pointer of some kind, then that would be
>>> simplest. But I think we could just do something like:
>>>
>>> else {
>>> delete(*it);
>>> cloned_buffers.erase(it);
>>> }
>> Ah that makes sense.
> Riki, I propose that you commit. Thanks for the fix.
Just to check: You've verified this fixes the problem?
Riki
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote:
> On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote:
> > On 2/18/20 6:07 PM, Scott Kostyshak wrote:
> > > Valgrind gave me the following error:
> > >
> > > ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely
> > > lost in loss record 5,165 of 5,862
> > > ==732==at 0x483AE63: operator new(unsigned long) (in
> > > /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> > > ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const
> > > (Buffer.cpp:661)
> > > ==732==by 0x11E583C: lyx::(anonymous
> > > namespace)::copyToTempBuffer(lyx::ParagraphList const&,
> > > std::shared_ptr) (CutAndPaste.cpp:582)
> > > ==732==by 0x11E5C6A: lyx::(anonymous
> > > namespace)::putClipboard(lyx::ParagraphList const&,
> > > std::shared_ptr,
> > > std::__cxx11::basic_string,
> > > std::allocator > const&, lyx::BufferParams) (CutAndPaste.cpp:613)
> > > ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&,
> > > std::__cxx11::basic_string,
> > > std::allocator > const&) (CutAndPaste.cpp:1123)
> > > ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&)
> > > (CutAndPaste.cpp:1024)
> > > ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&,
> > > lyx::FuncRequest&) (Text3.cpp:1593)
> > > ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&,
> > > lyx::FuncRequest&) (InsetText.cpp:339)
> > > ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&,
> > > lyx::FuncRequest&) (Inset.cpp:325)
> > > ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest const&)
> > > (Cursor.cpp:825)
> > > ==732==by 0x1816F4F:
> > > lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&,
> > > lyx::DispatchResult&) (GuiView.cpp:3878)
> > > ==732==by 0x181B959:
> > > lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&,
> > > lyx::DispatchResult&) (GuiView.cpp:4569)
> > >
> > > It comes from the following line (Buffer.cpp:661):
> > >
> > > cloned_buffers.push_back(new CloneList);
> > >
> > > Currently cloned_buffers is a list. Would it make sense to
> > > make it a list of *smart* pointers instead? Alternatively we could make a
> > > class and then make a custom destructor that would free the CloneLists
> > > that the list elements point to?
> >
> > This is some kind of thinko, probably on my part. The code at line 549
> > was supposed to be cleaning this up, but it actually only removes the
> > entry from the list.
> >
> > If it works to make it a smart pointer of some kind, then that would be
> > simplest. But I think we could just do something like:
> >
> > else {
> > delete(*it);
> > cloned_buffers.erase(it);
> > }
>
> Ah that makes sense.
Riki, I propose that you commit. Thanks for the fix.
Scott
signature.asc
Description: PGP signature
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On Wed, Feb 19, 2020 at 08:52:18AM +, Neven Sajko wrote: > > Well, actually the hardest part is waiting because LyX is very slow when > > run under valgrind. > > Try sanitizers instead. They are instrumentation that GCC or Clang can > include in executables. They do basically the same thing as Valgrind, > but should be much faster and since you are already compiling your own > Lyx and Qt, the sanitizers should be a more appropriate tool. There I do not always compile Qt. On my last attempt to compile Qt I came across an error and just used the pre-built libraries (and I think the corresponding -dbg packages). I did not initially realize that compiling Qt with the address sanitizers would make a big difference, but now I think I see the intuition: LyX might be responsible for a sanitizer error at the Qt level, and we would only catch that if we compile Qt with the address sanitizers. > are four sanitizers that you may want to compile with: asan, tsan, > msan, ubsan; each for a different class of bugs. > > Instructions: include -fsanitize=asan (replace asan with wanted > sanitizer if you want another one) to both compilation and linking > commands (GCC or Clang) of all libraries and code. It is also a good > idea to disable some compiler optimizations, in particular one should > use -fno-omit-frame-pointer. See > https://github.com/google/sanitizers/wiki for more information. > > By default the error reports will be output to stderr, but it is > possible to output them to a file by setting an environment variable > (search for log_path in > https://github.com/google/sanitizers/wiki/AddressSanitizerFlags if in > need of that option). > > I hope that was helpful. Thanks, Neven! This is helpful. I've been meaning to look into tools like this for a while, and this might be just the push I needed. Eventually I would like to add these options to the building and testing scripts at https://github.com/scottkosty/lyx-tester. It would be interesting to build with a sanitizer and then run our big suite of automated tests. I made an issue to remind me to look into this, although I probably won't look into it soon (just because of time reasons and other pending tasks): https://github.com/scottkosty/lyx-tester/issues/1 Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
My instructions for the C compiler and linker command line were wrong: instead of -fsanitize=asan , use -fsanitize=address or -fsanitize=thread or -fsanitize=undefined or -fsanitize=memory . And, of course, include debugging symbols with "-g". Regards, Neven Sajko -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
> Well, actually the hardest part is waiting because LyX is very slow when run > under valgrind. Try sanitizers instead. They are instrumentation that GCC or Clang can include in executables. They do basically the same thing as Valgrind, but should be much faster and since you are already compiling your own Lyx and Qt, the sanitizers should be a more appropriate tool. There are four sanitizers that you may want to compile with: asan, tsan, msan, ubsan; each for a different class of bugs. Instructions: include -fsanitize=asan (replace asan with wanted sanitizer if you want another one) to both compilation and linking commands (GCC or Clang) of all libraries and code. It is also a good idea to disable some compiler optimizations, in particular one should use -fno-omit-frame-pointer. See https://github.com/google/sanitizers/wiki for more information. By default the error reports will be output to stderr, but it is possible to output them to a file by setting an environment variable (search for log_path in https://github.com/google/sanitizers/wiki/AddressSanitizerFlags if in need of that option). I hope that was helpful. Regards, Neven Sajko -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote:
> On 2/18/20 6:07 PM, Scott Kostyshak wrote:
> > Valgrind gave me the following error:
> >
> > ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely
> > lost in loss record 5,165 of 5,862
> > ==732==at 0x483AE63: operator new(unsigned long) (in
> > /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> > ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const
> > (Buffer.cpp:661)
> > ==732==by 0x11E583C: lyx::(anonymous
> > namespace)::copyToTempBuffer(lyx::ParagraphList const&,
> > std::shared_ptr) (CutAndPaste.cpp:582)
> > ==732==by 0x11E5C6A: lyx::(anonymous
> > namespace)::putClipboard(lyx::ParagraphList const&,
> > std::shared_ptr,
> > std::__cxx11::basic_string,
> > std::allocator > const&, lyx::BufferParams) (CutAndPaste.cpp:613)
> > ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&,
> > std::__cxx11::basic_string,
> > std::allocator > const&) (CutAndPaste.cpp:1123)
> > ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&)
> > (CutAndPaste.cpp:1024)
> > ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&,
> > lyx::FuncRequest&) (Text3.cpp:1593)
> > ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&,
> > lyx::FuncRequest&) (InsetText.cpp:339)
> > ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&,
> > lyx::FuncRequest&) (Inset.cpp:325)
> > ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest const&)
> > (Cursor.cpp:825)
> > ==732==by 0x1816F4F:
> > lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&,
> > lyx::DispatchResult&) (GuiView.cpp:3878)
> > ==732==by 0x181B959:
> > lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&,
> > lyx::DispatchResult&) (GuiView.cpp:4569)
> >
> > It comes from the following line (Buffer.cpp:661):
> >
> > cloned_buffers.push_back(new CloneList);
> >
> > Currently cloned_buffers is a list. Would it make sense to
> > make it a list of *smart* pointers instead? Alternatively we could make a
> > class and then make a custom destructor that would free the CloneLists that
> > the list elements point to?
>
> This is some kind of thinko, probably on my part. The code at line 549
> was supposed to be cleaning this up, but it actually only removes the
> entry from the list.
>
> If it works to make it a smart pointer of some kind, then that would be
> simplest. But I think we could just do something like:
>
> else {
> delete(*it);
> cloned_buffers.erase(it);
> }
Ah that makes sense.
> I should learn how to use valgrind. Then I could solve these problems
> myself
The hard part (for me) is interpreting the results. Well, actually the
hardest part is waiting because LyX is very slow when run under
valgrind. Probably there are some options to mess with to speed things
up a bit. The command I run is this:
valgrind --leak-check=full --track-origins=yes --log-file=valgrind.log
~/lyxbuilds/master/CMakeBuild/bin/lyx -userdir ~/lyxbuilds/master/user-dir
Scott
signature.asc
Description: PGP signature
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On 2/18/20 6:07 PM, Scott Kostyshak wrote:
> Valgrind gave me the following error:
>
> ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely lost
> in loss record 5,165 of 5,862
> ==732==at 0x483AE63: operator new(unsigned long) (in
> /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const
> (Buffer.cpp:661)
> ==732==by 0x11E583C: lyx::(anonymous
> namespace)::copyToTempBuffer(lyx::ParagraphList const&,
> std::shared_ptr) (CutAndPaste.cpp:582)
> ==732==by 0x11E5C6A: lyx::(anonymous
> namespace)::putClipboard(lyx::ParagraphList const&,
> std::shared_ptr,
> std::__cxx11::basic_string,
> std::allocator > const&, lyx::BufferParams) (CutAndPaste.cpp:613)
> ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&,
> std::__cxx11::basic_string,
> std::allocator > const&) (CutAndPaste.cpp:1123)
> ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&)
> (CutAndPaste.cpp:1024)
> ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&,
> lyx::FuncRequest&) (Text3.cpp:1593)
> ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&,
> lyx::FuncRequest&) (InsetText.cpp:339)
> ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&,
> lyx::FuncRequest&) (Inset.cpp:325)
> ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest const&)
> (Cursor.cpp:825)
> ==732==by 0x1816F4F:
> lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&,
> lyx::DispatchResult&) (GuiView.cpp:3878)
> ==732==by 0x181B959: lyx::frontend::GuiView::dispatch(lyx::FuncRequest
> const&, lyx::DispatchResult&) (GuiView.cpp:4569)
>
> It comes from the following line (Buffer.cpp:661):
>
> cloned_buffers.push_back(new CloneList);
>
> Currently cloned_buffers is a list. Would it make sense to make
> it a list of *smart* pointers instead? Alternatively we could make a class
> and then make a custom destructor that would free the CloneLists that the
> list elements point to?
This is some kind of thinko, probably on my part. The code at line 549
was supposed to be cleaning this up, but it actually only removes the
entry from the list.
If it works to make it a smart pointer of some kind, then that would be
simplest. But I think we could just do something like:
else {
delete(*it);
cloned_buffers.erase(it);
}
I should learn how to use valgrind. Then I could solve these problems
myself
Riki
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Memory leak from list
Valgrind gave me the following error: ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely lost in loss record 5,165 of 5,862 ==732==at 0x483AE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const (Buffer.cpp:661) ==732==by 0x11E583C: lyx::(anonymous namespace)::copyToTempBuffer(lyx::ParagraphList const&, std::shared_ptr) (CutAndPaste.cpp:582) ==732==by 0x11E5C6A: lyx::(anonymous namespace)::putClipboard(lyx::ParagraphList const&, std::shared_ptr, std::__cxx11::basic_string, std::allocator > const&, lyx::BufferParams) (CutAndPaste.cpp:613) ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&, std::__cxx11::basic_string, std::allocator > const&) (CutAndPaste.cpp:1123) ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&) (CutAndPaste.cpp:1024) ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&, lyx::FuncRequest&) (Text3.cpp:1593) ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&, lyx::FuncRequest&) (InsetText.cpp:339) ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&, lyx::FuncRequest&) (Inset.cpp:325) ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest const&) (Cursor.cpp:825) ==732==by 0x1816F4F: lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&, lyx::DispatchResult&) (GuiView.cpp:3878) ==732==by 0x181B959: lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&, lyx::DispatchResult&) (GuiView.cpp:4569) It comes from the following line (Buffer.cpp:661): cloned_buffers.push_back(new CloneList); Currently cloned_buffers is a list. Would it make sense to make it a list of *smart* pointers instead? Alternatively we could make a class and then make a custom destructor that would free the CloneLists that the list elements point to? Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
