Bug Hunting Session
Bug 99352 - Some VclPtrs leak past DeInitVCL
Summary: Some VclPtrs leak past DeInitVCL
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:5.2.0 target:5.4.0
Keywords:
Depends on:
Blocks: VclPtr
  Show dependency treegraph
 
Reported: 2016-04-16 19:19 UTC by Björn Michaelsen
Modified: 2017-04-04 15:54 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments
patch to look for leaked VclPtrs at the end of DeInitVCL (3.31 KB, patch)
2016-04-16 19:19 UTC, Björn Michaelsen
Details
patch to look for leaked VclPtrs at the end of DeInitVCL (3.32 KB, text/plain)
2016-04-16 22:48 UTC, Björn Michaelsen
Details
patch to look for leaked VclPtrs at the end of DeInitVCL (against 14b4fcf0) (3.58 KB, patch)
2017-03-01 19:53 UTC, Björn Michaelsen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Björn Michaelsen 2016-04-16 19:19:23 UTC
Created attachment 124413 [details]
patch to look for leaked VclPtrs at the end of DeInitVCL

Some VclPtrs leak past the end of DeInitVCL, which is unfortunate as they might still be subject to callback and assume VCL to be alive.

Applying the attached patch and running:

 make subsequencheck; find workdir/JunitTest/ -name '*log'|xargs grep LEAK|sed -e 's/^.*LEAK/LEAK/'|sort|uniq -c

result in:

      1 LEAKED VCLPTR: P10SfxPrinter refered to from a PK6VclPtrI10SfxPrinterE
      1 LEAKED VCLPTR: P12OpenGLWindow refered to from a PK6VclPtrI12OpenGLWindowE
     27 LEAKED VCLPTR: P12OutputDevice refered to from a PK6VclPtrI12OutputDeviceE
     14 LEAKED VCLPTR: P13VirtualDevice refered to from a PK6VclPtrI13VirtualDeviceE
      2 LEAKED VCLPTR: PN3vcl6WindowE refered to from a PK6VclPtrIN3vcl6WindowEE
      1 LEAKED VCLPTR: PN5chart11ChartWindowE refered to from a PK6VclPtrIN5chart11ChartWindowEE

It would be good to have all those eliminated and explicitly have them set to nullptrs deterministically in DeInitVCL (or earlier).

Additional notes:
- Some VclPtr have a vptr of NULL while still alive, which is discomforting in itself
- Doing a build with the patch and e.g. playing through MozTrap testcases manually to find well-reproducable leaks likely is a nice EasyHack
Comment 1 Björn Michaelsen 2016-04-16 19:42:04 UTC
Whoops, there is more than JunitTests. This:

  find workdir/JunitTest/ workdir/CppunitTest/ workdir/PythonTest/ -name '*log'|xargs grep LEAK|sed -e 's/^.*LEAK/LEAK/'|sort|uniq -c|sort -n

yields:

      1 Binary file workdir/CppunitTest/xmlsecurity_signing.test.log matches
      1 Binary file workdir/JunitTest/unordf_complex/done.log matches
      1 LEAKED VCLPTR: P12OpenGLWindow refered to from a PK6VclPtrI12OpenGLWindowE
      1 LEAKED VCLPTR: P12ScGridWindow refered to from a PK6VclPtrI12ScGridWindowE
      1 LEAKED VCLPTR: P12ScTabControl refered to from a PK6VclPtrI12ScTabControlE
      1 LEAKED VCLPTR: P13ImplTabButton refered to from a PK6VclPtrI13ImplTabButtonE
      1 LEAKED VCLPTR: P13ScTabSplitter refered to from a PK6VclPtrI13ScTabSplitterE
      1 LEAKED VCLPTR: P14ScCornerButton refered to from a PK6VclPtrI14ScCornerButtonE
      1 LEAKED VCLPTR: P14SfxSplitWindow refered to from a PK6VclPtrI14SfxSplitWindowE
      1 LEAKED VCLPTR: P21SfxEmptySplitWin_Impl refered to from a PK6VclPtrI21SfxEmptySplitWin_ImplE
      1 LEAKED VCLPTR: P8ScColBar refered to from a PK6VclPtrI8ScColBarE
      1 LEAKED VCLPTR: P8ScRowBar refered to from a PK6VclPtrI8ScRowBarE
      1 LEAKED VCLPTR: PN5chart11ChartWindowE refered to from a PK6VclPtrIN5chart11ChartWindowEE
      2 LEAKED VCLPTR: P10PushButton refered to from a PK6VclPtrI10PushButtonE
      2 LEAKED VCLPTR: P10TextWindow refered to from a PK6VclPtrI10TextWindowE
      2 LEAKED VCLPTR: P11DecoToolBox refered to from a PK6VclPtrI11DecoToolBoxE
      2 LEAKED VCLPTR: P13SvTreeListBox refered to from a PK6VclPtrI13SvTreeListBoxE
      2 LEAKED VCLPTR: P16ExtMultiLineEdit refered to from a PK6VclPtrI16ExtMultiLineEditE
      2 LEAKED VCLPTR: P16VclMultiLineEdit refered to from a PK6VclPtrI16VclMultiLineEditE
      2 LEAKED VCLPTR: P17SvtIconChoiceCtrl refered to from a PK6VclPtrI17SvtIconChoiceCtrlE
      2 LEAKED VCLPTR: P7ToolBox refered to from a PK6VclPtrI7ToolBoxE
      2 LEAKED VCLPTR: P8Splitter refered to from a PK6VclPtrI8SplitterE
      2 LEAKED VCLPTR: P9FixedLine refered to from a PK6VclPtrI9FixedLineE
      2 LEAKED VCLPTR: P9FixedText refered to from a PK6VclPtrI9FixedTextE
      2 LEAKED VCLPTR: P9StatusBar refered to from a PK6VclPtrI9StatusBarE
      2 LEAKED VCLPTR: PN5dbaui12OTitleWindowE refered to from a PK6VclPtrIN5dbaui12OTitleWindowEE
      2 LEAKED VCLPTR: PN5dbaui13OCreationListE refered to from a PK6VclPtrIN5dbaui13OCreationListEE
      2 LEAKED VCLPTR: PN5dbaui14OPreviewWindowE refered to from a PK6VclPtrIN5dbaui14OPreviewWindowEE
      2 LEAKED VCLPTR: PN5dbaui16OAppBorderWindowE refered to from a PK6VclPtrIN5dbaui16OAppBorderWindowEE
      2 LEAKED VCLPTR: PN5dbaui16OApplicationViewE refered to from a PK6VclPtrIN5dbaui16OApplicationViewEE
      2 LEAKED VCLPTR: PN5dbaui20OAppDetailPageHelperE refered to from a PK6VclPtrIN5dbaui20OAppDetailPageHelperEE
      2 LEAKED VCLPTR: PN5dbaui22OApplicationDetailViewE refered to from a PK6VclPtrIN5dbaui22OApplicationDetailViewEE
      2 LEAKED VCLPTR: PN5dbaui23OApplicationIconControlE refered to from a PK6VclPtrIN5dbaui23OApplicationIconControlEE
      2 LEAKED VCLPTR: PN5dbaui9ODataViewE refered to from a PK6VclPtrIN5dbaui9ODataViewEE
      2 LEAKED VCLPTR: PN7svtools20ODocumentInfoPreviewE refered to from a PK6VclPtrIN7svtools20ODocumentInfoPreviewEE
      3 LEAKED VCLPTR: P10SfxPrinter refered to from a PK6VclPtrI10SfxPrinterE
      3 LEAKED VCLPTR: P12ScrollBarBox refered to from a PK6VclPtrI12ScrollBarBoxE
      3 LEAKED VCLPTR: P16ImplBorderWindow refered to from a PK6VclPtrI16ImplBorderWindowE
      3 LEAKED VCLPTR: P9ScrollBar refered to from a PK6VclPtrI9ScrollBarE
     64 LEAKED VCLPTR: P13VirtualDevice refered to from a PK6VclPtrI13VirtualDeviceE
     71 LEAKED VCLPTR: PN3vcl6WindowE refered to from a PK6VclPtrIN3vcl6WindowEE
     99 LEAKED VCLPTR: P12OutputDevice refered to from a PK6VclPtrI12OutputDeviceE
Comment 2 Björn Michaelsen 2016-04-16 22:48:30 UTC
Created attachment 124416 [details]
patch to look for leaked VclPtrs at the end of DeInitVCL

(bumped diff)
Comment 3 Noel Grandin 2016-04-17 06:40:47 UTC
I can't see us fixing all of those, unless they are actually referenced post-deinit.

I am however working on a patch to clean up the statically allocate VclPtr<> vars.
Comment 4 Björn Michaelsen 2016-04-17 10:07:18 UTC
(In reply to Noel Grandin from comment #3)
> I can't see us fixing all of those, unless they are actually referenced
> post-deinit.

I just wonder: Could we have a clang plugin that rewrites VclPtr::operator=() to something VclPtr::assign(ptr, __FILE__, __LINE__) so we can register where a VclPtr was assigned in that global registry? We could then output something like:

LEAKED VCLPTR: P12OutputDevice refered to from a PK6VclPtrI12OutputDeviceE assigned at vcl/foo/bar.cxx:564

which should make hunting down these much easier ...
Comment 5 Noel Grandin 2016-04-17 10:51:03 UTC
(In reply to Björn Michaelsen from comment #4)
> 
> I just wonder: Could we have a clang plugin that rewrites
> VclPtr::operator=() to something VclPtr::assign(ptr, __FILE__, __LINE__) so
> we can register where a VclPtr was assigned in that global registry? We
> could then output something like:
> 


Or we could use the nice stack backtrace functionality mmeeks built to store a backtrace in operator=, then symbolise the relevant ones.
Comment 6 Michael Meeks 2016-04-18 09:05:50 UTC
Oh ! the way to track vclptr leaks is using:

https://lists.freedesktop.org/archives/libreoffice/2015-May/068067.html

Which uses gdb to track all traces that mutate the reference count and then pairs them up - to throw out most of the noise and (in theory) - this shows you the one guy that unreffed or reffed too much ;-)

At least -that was my gdb assisted solution - that doesn't require code changes.

Prolly ought to be documented in vcl/README.lifecycle =)

Then again - I guess in DBGUTIL mode it'd be great to have something like your tracker included in master Bjoern - ultimately we should chase all of these down, although I suspect that we have a few known leaks still lingering in VCL.

Thanks for chasing this lot though ! =)
Comment 7 Commit Notification 2016-04-18 10:54:51 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=67d333c608a662621c1069aacdec75e45e33a183

tdf#99352 - Some VclPtrs leak past DeInitVCL

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 8 Björn Michaelsen 2016-04-19 15:34:36 UTC
Rerun against 024d2fde2aae13b07cf5c7b4d85fc3c6abce6913 yields:

      1 Binary file workdir/CppunitTest/xmlsecurity_signing.test.log matches
      1 LEAKED VCLPTR: P12OpenGLWindow refered to from a PK6VclPtrI12OpenGLWindowE
      1 LEAKED VCLPTR: P12ScGridWindow refered to from a PK6VclPtrI12ScGridWindowE
      1 LEAKED VCLPTR: P12ScTabControl refered to from a PK6VclPtrI12ScTabControlE
      1 LEAKED VCLPTR: P13ImplTabButton refered to from a PK6VclPtrI13ImplTabButtonE
      1 LEAKED VCLPTR: P13ScTabSplitter refered to from a PK6VclPtrI13ScTabSplitterE
      1 LEAKED VCLPTR: P14ScCornerButton refered to from a PK6VclPtrI14ScCornerButtonE
      1 LEAKED VCLPTR: P14SfxSplitWindow refered to from a PK6VclPtrI14SfxSplitWindowE
      1 LEAKED VCLPTR: P21SfxEmptySplitWin_Impl refered to from a PK6VclPtrI21SfxEmptySplitWin_ImplE
      1 LEAKED VCLPTR: P8ScColBar refered to from a PK6VclPtrI8ScColBarE
      1 LEAKED VCLPTR: P8ScRowBar refered to from a PK6VclPtrI8ScRowBarE
      1 LEAKED VCLPTR: PN5chart11ChartWindowE refered to from a PK6VclPtrIN5chart11ChartWindowEE
      2 LEAKED VCLPTR: P10PushButton refered to from a PK6VclPtrI10PushButtonE
      2 LEAKED VCLPTR: P10TextWindow refered to from a PK6VclPtrI10TextWindowE
      2 LEAKED VCLPTR: P11DecoToolBox refered to from a PK6VclPtrI11DecoToolBoxE
      2 LEAKED VCLPTR: P13SvTreeListBox refered to from a PK6VclPtrI13SvTreeListBoxE
      2 LEAKED VCLPTR: P16ExtMultiLineEdit refered to from a PK6VclPtrI16ExtMultiLineEditE
      2 LEAKED VCLPTR: P16VclMultiLineEdit refered to from a PK6VclPtrI16VclMultiLineEditE
      2 LEAKED VCLPTR: P17SvtIconChoiceCtrl refered to from a PK6VclPtrI17SvtIconChoiceCtrlE
      2 LEAKED VCLPTR: P7ToolBox refered to from a PK6VclPtrI7ToolBoxE
      2 LEAKED VCLPTR: P8Splitter refered to from a PK6VclPtrI8SplitterE
      2 LEAKED VCLPTR: P9FixedLine refered to from a PK6VclPtrI9FixedLineE
      2 LEAKED VCLPTR: P9FixedText refered to from a PK6VclPtrI9FixedTextE
      2 LEAKED VCLPTR: P9StatusBar refered to from a PK6VclPtrI9StatusBarE
      2 LEAKED VCLPTR: PN5dbaui12OTitleWindowE refered to from a PK6VclPtrIN5dbaui12OTitleWindowEE
      2 LEAKED VCLPTR: PN5dbaui13OCreationListE refered to from a PK6VclPtrIN5dbaui13OCreationListEE
      2 LEAKED VCLPTR: PN5dbaui14OPreviewWindowE refered to from a PK6VclPtrIN5dbaui14OPreviewWindowEE
      2 LEAKED VCLPTR: PN5dbaui16OAppBorderWindowE refered to from a PK6VclPtrIN5dbaui16OAppBorderWindowEE
      2 LEAKED VCLPTR: PN5dbaui16OApplicationViewE refered to from a PK6VclPtrIN5dbaui16OApplicationViewEE
      2 LEAKED VCLPTR: PN5dbaui20OAppDetailPageHelperE refered to from a PK6VclPtrIN5dbaui20OAppDetailPageHelperEE
      2 LEAKED VCLPTR: PN5dbaui22OApplicationDetailViewE refered to from a PK6VclPtrIN5dbaui22OApplicationDetailViewEE
      2 LEAKED VCLPTR: PN5dbaui23OApplicationIconControlE refered to from a PK6VclPtrIN5dbaui23OApplicationIconControlEE
      2 LEAKED VCLPTR: PN5dbaui9ODataViewE refered to from a PK6VclPtrIN5dbaui9ODataViewEE
      2 LEAKED VCLPTR: PN7svtools20ODocumentInfoPreviewE refered to from a PK6VclPtrIN7svtools20ODocumentInfoPreviewEE
      3 LEAKED VCLPTR: P10SfxPrinter refered to from a PK6VclPtrI10SfxPrinterE
      3 LEAKED VCLPTR: P12ScrollBarBox refered to from a PK6VclPtrI12ScrollBarBoxE
      3 LEAKED VCLPTR: P16ImplBorderWindow refered to from a PK6VclPtrI16ImplBorderWindowE
      3 LEAKED VCLPTR: P9ScrollBar refered to from a PK6VclPtrI9ScrollBarE
     42 LEAKED VCLPTR: P13VirtualDevice refered to from a PK6VclPtrI13VirtualDeviceE
     71 LEAKED VCLPTR: PN3vcl6WindowE refered to from a PK6VclPtrIN3vcl6WindowEE
     73 LEAKED VCLPTR: P12OutputDevice refered to from a PK6VclPtrI12OutputDeviceE

@Noel: Clearly an improvement, good job!
Comment 9 Xisco Faulí 2016-09-15 22:33:53 UTC
Hello,
Is this bug fixed?
If so, could you please close it as RESOLVED FIXED?
Comment 10 Björn Michaelsen 2017-03-01 11:55:01 UTC
(In reply to Xisco Faulí from comment #9)
> Is this bug fixed?
> If so, could you please close it as RESOLVED FIXED?

To find out the status quo, one would need to apply http://bugs.documentfoundation.org/attachment.cgi?id=124413 and run tests again to show what is left.

For real world impact, lp#1562971 was dominating the crash reports on Ubuntu for a while, but as said in https://bugs.launchpad.net/ubuntu/+source/libreoffice/+bug/1562971/comments/9 this particular stacktrace occured only twice on LibreOffice 5.2.x and has never been seen on LibreOffice 5.3.x so far.

I will try to rerun the attached patch in the next days, but so far it looks like WORKSFORME.
Comment 11 Björn Michaelsen 2017-03-01 19:53:52 UTC
Created attachment 131567 [details]
patch to look for leaked VclPtrs at the end of DeInitVCL (against 14b4fcf0)

rebased patch on 14b4fcf0.
Comment 12 Björn Michaelsen 2017-03-01 20:07:42 UTC
Here is the output of "find workdir/JunitTest/ workdir/CppunitTest/ workdir/PythonTest/ -name '*log'|xargs grep LEAK|sed -e 's/^.*LEAK/LEAK/'|sort|uniq -c|sort -n":

      1 LEAKED VCLPTR: P12OpenGLWindow refered to from a PK6VclPtrI12OpenGLWindowE
      1 LEAKED VCLPTR: PN5chart11ChartWindowE refered to from a PK6VclPtrIN5chart11ChartWindowEE
    156 LEAKED VCLPTR: P12OutputDevice refered to from a PK6VclPtrI12OutputDeviceE
    159 LEAKED VCLPTR: P13VirtualDevice refered to from a PK6VclPtrI13VirtualDeviceE
    260 LEAKED VCLPTR: 

which is ... disappointing:

1/ apparently there are lots of VclPtrs without typeinfo hanging around (or are these just nullptrs?)
2/ the VirtualDevice<->VirtualDevice pointers increased from 64 to 159 since Comment 1
3/ the OutputDevice<->OutputDevice pointers increased from 99 to 156 since Comment 1
4/ the chart::ChartWindow<->chart::ChartWindow pointer is new

On the good news: Most of the rest is gone (which is good, if the 260 are indeed nullptrs).

Notes:
- This run excludes CppunitTest_sw_ooxmlencryption and CppunitTest_xmlsecurity_pdfsigning which segfault against the nss (3.29.1) on my machine.
- This run also excludes CppunitTest_vcl_wmf_test which crashes on a our_vAllVclPtrs.remove(this) on exit, which shouldnt happen.
- This run also excludes CppunitTest_sw_mailmerge which might be looping (or excessively slow). I killed it after some 10 CPU minutes.
Comment 13 Björn Michaelsen 2017-03-01 20:17:20 UTC
(In reply to Björn Michaelsen from comment #12)
> or are these just nullptrs?

Eh, yes. So this is good.

Looking at individual tests:
>       1 LEAKED VCLPTR: PN5chart11ChartWindowE refered to from a
> PK6VclPtrIN5chart11ChartWindowEE

That is in JunitTest/chart2_unoapi.

Most other tests leak 1 OutputDevice, 1 VirtualDevice and 2 nullptrs (or less).

Exceptions leaking more (ignoring nullptrs):

workdir/JunitTest/sw_complex/done.log
      3 LEAKED VCLPTR: P12OutputDevice refered to from a PK6VclPtrI12OutputDeviceE
      3 LEAKED VCLPTR: P13VirtualDevice refered to from a PK6VclPtrI13VirtualDeviceE
workdir/JunitTest/sfx2_complex/done.log
      4 LEAKED VCLPTR: P12OutputDevice refered to from a PK6VclPtrI12OutputDeviceE
      4 LEAKED VCLPTR: P13VirtualDevice refered to from a PK6VclPtrI13VirtualDeviceE
workdir/JunitTest/framework_complex/done.log
      2 LEAKED VCLPTR: P12OutputDevice refered to from a PK6VclPtrI12OutputDeviceE
      2 LEAKED VCLPTR: P13VirtualDevice refered to from a PK6VclPtrI13VirtualDeviceE
Comment 14 Björn Michaelsen 2017-03-01 20:30:06 UTC
(In reply to Björn Michaelsen from comment #13)
> Looking at individual tests:
> >       1 LEAKED VCLPTR: PN5chart11ChartWindowE refered to from a
> > PK6VclPtrIN5chart11ChartWindowEE
> 
> That is in JunitTest/chart2_unoapi.

And the:
>       1 LEAKED VCLPTR: P12OpenGLWindow refered to from a PK6VclPtrI12OpenGLWindowE

is also from chart2_unoapi.
Comment 15 Björn Michaelsen 2017-03-01 21:15:11 UTC
(In reply to Björn Michaelsen from comment #13)
> workdir/JunitTest/sw_complex/done.log
> workdir/JunitTest/sfx2_complex/done.log
> workdir/JunitTest/framework_complex/done.log

Rerunning these standalone show the 1 OutputDevice, 1 VirtualDevice from other tests. Curious.

(In reply to Björn Michaelsen from comment #12)
> - This run also excludes CppunitTest_sw_mailmerge which might be looping (or
> excessively slow). I killed it after some 10 CPU minutes.

Rerunning standalone showed no issues. Thus:

> - This run also excludes CppunitTest_vcl_wmf_test which crashes on a our_vAllVclPtrs.remove(this) on exit, which shouldnt happen.

and chart2_unoapi leaking OpenGL and a window. are the only remaining issue. Once that is fixed, we _could_ think about adding the attached patch to debug builds and make those barf up if more than the expected 1 OutputDevice, 1 VirtualDevice are left over. This should help detect regressions early.
Comment 16 Björn Michaelsen 2017-03-02 01:32:31 UTC
      1 LEAKED VCLPTR: P12OpenGLWindow created at unknown line -1 refered to from a PK6VclPtrI12OpenGLWindowE
      1 LEAKED VCLPTR: P13VirtualDevice created at svx/source/svdraw/sdrpaintwindow.cxx line 112 refered to from a PK6VclPtrI13VirtualDeviceE
      1 LEAKED VCLPTR: PN5chart11ChartWindowE created at chart2/source/controller/main/ChartController.cxx line 453 refered to from a PK6VclPtrIN5chart11ChartWindowEE
     15 LEAKED VCLPTR: P13VirtualDevice created at editeng/source/editeng/impedit2.cxx line 213 refered to from a PK6VclPtrI13VirtualDeviceE
    156 LEAKED VCLPTR: P13VirtualDevice created at editeng/source/editeng/eerdll.cxx line 89 refered to from a PK6VclPtrI13VirtualDeviceE
    159 LEAKED VCLPTR: P12OutputDevice created at unknown line -1 refered to from a PK6VclPtrI12OutputDeviceE
    161 LEAKED VCLPTR: P13VirtualDevice created at unknown line -1 refered to from a PK6VclPtrI13VirtualDeviceE

so it seems the stuff we are leaking are:
- EditEngine
- chart::ChartController

The VirtualDevices/OutputDevices likely will dispose along with a proper EditEngine shutdown as the leak counts match up pretty well.
Comment 17 Commit Notification 2017-03-03 10:23:49 UTC
Bjoern Michaelsen committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=926c0d73e82bb1d5644c49fb95c56e241b6c8f34

tdf#99352: ensure ChartController is disposing its VclPtrs on DeInitVCL

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 18 Michael Meeks 2017-03-03 10:25:21 UTC
Hmm; not really a VclPtr bug surely - as all of these guys would have been leaks in the past anyway ;-) but fair enough to track it there I guess. Nice work !
Comment 19 Commit Notification 2017-03-05 08:49:12 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=60708af4a7ed48028cf3158d1101568aa5b0da81

Revert "tdf#99352: ensure ChartController is disposing its VclPtrs on DeInitVCL"

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 20 Björn Michaelsen 2017-03-05 23:35:43 UTC
So with: changeset https://gerrit.libreoffice.org/34883, https://gerrit.libreoffice.org/34877 and https://gerrit.libreoffice.org/34841, and some more info on where these leaking pointers where created this is down to:

      1 LEAKED VCLPTR: P12OpenGLWindow created at OPERATOR= line -1 referred to from a PK6VclPtrI12OpenGLWindowE
      1 LEAKED VCLPTR: P12OutputDevice created at chart2/source/view/main/DrawModelWrapper.cxx line 114 referred to from a PK6VclPtrI12OutputDeviceE
      1 LEAKED VCLPTR: P13VirtualDevice created at svx/source/svdraw/sdrpaintwindow.cxx line 112 referred to from a PK6VclPtrI13VirtualDeviceE
     33 LEAKED VCLPTR: P13VirtualDevice created at RESET line -1 referred to from a PK6VclPtrI13VirtualDeviceE
     50 LEAKED VCLPTR: P13VirtualDevice created at unknown line -1 referred to from a PK6VclPtrI13VirtualDeviceE
    120 LEAKED VCLPTR: P12OutputDevice created at unknown line -1 referred to from a PK6VclPtrI12OutputDeviceE
    260 LEAKED VCLPTR: 

That is down from 317 non-null VclPtrs on deinit in Comment 12 to 206 non-null VclPtrs (down by 35%).
Comment 21 Björn Michaelsen 2017-03-06 01:18:53 UTC
(In reply to Commit Notification from comment #19)
> Markus Mohrhard committed a patch related to this issue.
> http://cgit.freedesktop.org/libreoffice/core/commit/
> ?id=60708af4a7ed48028cf3158d1101568aa5b0da81
> 
> Revert "tdf#99352: ensure ChartController is disposing its VclPtrs on
> DeInitVCL"
> 
> The original patch crashes charts on disposing, e.g. during the UI testing.

@moggi: Do you have a reproduction scenario for that? Some random manual teasing of chart2 didnt immediately yield anything, so something specific would be helpful.
Comment 22 Markus Mohrhard 2017-03-06 07:50:05 UTC
(In reply to Björn Michaelsen from comment #21)
> (In reply to Commit Notification from comment #19)
> > Markus Mohrhard committed a patch related to this issue.
> > http://cgit.freedesktop.org/libreoffice/core/commit/
> > ?id=60708af4a7ed48028cf3158d1101568aa5b0da81
> > 
> > Revert "tdf#99352: ensure ChartController is disposing its VclPtrs on
> > DeInitVCL"
> > 
> > The original patch crashes charts on disposing, e.g. during the UI testing.
> 
> @moggi: Do you have a reproduction scenario for that? Some random manual
> teasing of chart2 didnt immediately yield anything, so something specific
> would be helpful.

Make UITest_calc_demo or follow the steps from calc_test.edit_chart.test_select_secondary_axis
Even if I follow the steps manually it crashes in the last deselection of the chart.
Comment 23 Commit Notification 2017-03-10 13:01:19 UTC
Bjoern Michaelsen committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1a044b25c279236d9f67847ec6ad426d8c5aac13

tdf#99352: dispose EditEngines when SfxApp dies

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 24 Commit Notification 2017-03-10 13:03:47 UTC
Bjoern Michaelsen committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=c54bb4a9f76a11561a7f4010382dbe46c0d2ef2a

tdf#99352: create editeng::SharedVclRessources

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 25 Buovjaga 2017-03-10 14:47:34 UTC
I guess related to the previous commits, I now get a segfault on CppunitTest/libtest_editeng_borderline.so when building.
Comment 26 Buovjaga 2017-03-10 15:24:38 UTC
(In reply to Buovjaga from comment #25)
> I guess related to the previous commits, I now get a segfault on
> CppunitTest/libtest_editeng_borderline.so when building.

Apparently fixed by https://cgit.freedesktop.org/libreoffice/core/commit/?id=d7685eb3f417e19a5e61f2fe550eb03d6848365d
Comment 27 Björn Michaelsen 2017-03-27 08:06:35 UTC
(In reply to Buovjaga from comment #26)
> Apparently fixed by
> https://cgit.freedesktop.org/libreoffice/core/commit/
> ?id=d7685eb3f417e19a5e61f2fe550eb03d6848365d

Yes, that were two incompatible, but non-conflicting (in a git sense) changes between my change on gerrit and master between when I pushed the patch for review and when it was cherry-picked.

Anyway, with https://gerrit.libreoffice.org/#/c/35690/ this is down to 173 non-null leaked VclPtrs:
     56 LEAKED VCLPTR: P13VirtualDevice refered to from a PK6VclPtrI13VirtualDeviceE
    118 LEAKED VCLPTR: P12OutputDevice refered to from a PK6VclPtrI12OutputDeviceE
    257 LEAKED VCLPTR:
Comment 28 Commit Notification 2017-04-04 14:36:21 UTC
Bjoern Michaelsen committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=3ecb9b4bd7dc70664bbb8d7c957ea8dc5015223f

tdf#99352: assert on stray VclPtrs past DeInit

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 29 Björn Michaelsen 2017-04-04 15:54:23 UTC
(In reply to Commit Notification from comment #28)
> http://cgit.freedesktop.org/libreoffice/core/commit/
> ?id=3ecb9b4bd7dc70664bbb8d7c957ea8dc5015223f
> 
> tdf#99352: assert on stray VclPtrs past DeInit

And with that: Closing this one as from now on DBG_UTL builds (except on Windows) will barf up on any leaked VclPtr, thus we should notice really quickly if we regress here.