Bug Hunting Session
Bug 38590 - Possible fd-leak with speadsheet containing Chart objects
Summary: Possible fd-leak with speadsheet containing Chart objects
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
3.4.1 RC1
Hardware: x86-64 (AMD64) Linux (All)
: highest blocker
Assignee: Not Assigned
URL:
Whiteboard:
Keywords:
: 38507 38587 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-06-23 00:36 UTC by Jean-Baptiste Faure
Modified: 2011-10-29 07:38 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments
spreadsheet with charts which cause probable memory leak (97.67 KB, application/vnd.oasis.opendocument.spreadsheet)
2011-06-23 00:36 UTC, Jean-Baptiste Faure
Details
another test document (18.24 KB, application/vnd.oasis.opendocument.spreadsheet)
2011-06-23 07:42 UTC, Petr Mladek
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jean-Baptiste Faure 2011-06-23 00:36:00 UTC
Created attachment 48323 [details]
spreadsheet with charts which cause probable memory leak

Steps to reproduce:
- open the attached spreadsheet with LibO 3.4.1 rc1
- go to the bottom of the sheet to show the charts
- navigate in the sheet several times in order to show and hide the charts
- after a moment it becomes impossible to save the file : too many files open !

In fact, if you look at the temporary folder used by LibO, you can see that Calc create many new temp files each time a chart becomes visible.
With the attached file I reach quickly ~1000 files
result of ulimit -n command : 1024 ...

Best regards. JBF
Comment 1 Jean-Baptiste Faure 2011-06-23 00:43:32 UTC
correction of summary.
Consequence of this bug : data loss when you can't save your file.
Comment 2 Markus Mohrhard 2011-06-23 01:14:23 UTC
possible duplicate of https://bugs.freedesktop.org/show_bug.cgi?id=38507

I haven't looked into it but sounds similar.
Comment 3 Petr Mladek 2011-06-23 07:42:42 UTC
Created attachment 48343 [details]
another test document

It might be related to charts. I have created another sample document with two charts. It opens new temp files every time I go down or up the page.

The temp files are binary files starting with the text "VCLMTF". I think that it is a meta file describing the chart picture. I guess that it draws the chart from the beginning everytime it appears on the screen. It does not reuse and and does not close pictures that are not longer visible.
Comment 4 Petr Mladek 2011-06-23 08:48:57 UTC
I do not see it with 3.4.0, so it a regression added in this bug fix release :-(
Comment 5 Jean-Baptiste Faure 2011-06-23 10:37:57 UTC
(In reply to comment #4)
> I do not see it with 3.4.0, so it a regression added in this bug fix release
> :-(

Something has been changed between 3.4.0 and 3.4.1 in the updating of a chart when data used to define this chart are modified. In 3.4.0 when you modify the value of a data, the chart is not updated automatically. It is in 3.4.1 rc1.

Best regards. JBF
Comment 6 Michael Meeks 2011-06-23 10:49:32 UTC
I built the latest libreoffice-3-4 which shows the problem, then I downgraded -only- the 'sc' module to libreoffice-3-4-0 and the problem is gone.

Ergo at least the exacerbating cause of the issue is in the 'calc' repository (which includes chart2 of course) between libreoffice-3-4-0 and libreoffice-3-4-1. Hopefully that narrows it a little.
Comment 7 Michael Meeks 2011-06-23 11:00:30 UTC
I didn't like this patch at the time it was reviewed on the list ;-> so I was suspicious from the outset that this was it, and ... bingo :-)

If I add / and/or remove just this one commit:

commit 70af08e849a93d56915b7abe14537facf1022a6a
Author: Cédric Bosdonnat <cedric.bosdonnat.ooo@free.fr>
Date:   Wed May 18 12:04:28 2011 +0200

    fdo#36688: Fixed undisplayed calc page and header / footer borders
    
    When displaying the page, header and footer borders, calc uses a fake
    ScDocument with no drawing page. Create a new SdrPage if none can be
    fetched from the ScDocument to create the primitive processor.
    
    Signed-off-by: Kohei Yoshida <kyoshida@novell.com>

diff --git a/sc/source/ui/view/output.cxx b/sc/source/ui/view/output.cxx
index f37a2c8..22b5d24 100644
--- a/sc/source/ui/view/output.cxx
+++ b/sc/source/ui/view/output.cxx
@@ -52,6 +52,7 @@
 #include <basegfx/matrix/b2dhommatrix.hxx>
 #include <svx/sdr/contact/objectcontacttools.hxx>
 #include <svx/unoapi.hxx>
+#include <svx/svdpage.hxx>
 
 #include "output.hxx"
 #include "document.hxx"
@@ -1634,17 +1635,19 @@ void ScOutputData::DrawRotatedFrame( const Color* pForceColor )
 
 drawinglayer::processor2d::BaseProcessor2D* ScOutputData::CreateProcessor2D( )
 {
+    SdrModel aModel;
+    SdrPage aSdrPage( aModel );
+
     ScDrawLayer* pDrawLayer = pDoc->GetDrawLayer();
-    if (!pDrawLayer)
-        return NULL;
+    if ( pDrawLayer )
+        aSdrPage = *pDrawLayer->GetPage( static_cast< sal_uInt16 >( nTab ) );
 
     basegfx::B2DRange aViewRange;
-    SdrPage *pDrawPage = pDrawLayer->GetPage( static_cast< sal_uInt16 >( nTab ) );
     const drawinglayer::geometry::ViewInformation2D aNewViewInfos(
...

Then the problem appears and/or goes away.

Creating that new SdrModel that new SdrPage and what we then do with it always seemed highly suspect to me :-) Of course, this prolly points to an underlying leak anyway - but we provoke it regularly here.
Comment 8 Michael Meeks 2011-06-23 11:01:59 UTC
Cedric - your patch ...
Comment 9 Katarina Behrens 2011-06-23 11:40:42 UTC
*** Bug 38507 has been marked as a duplicate of this bug. ***
Comment 10 cunio 2011-06-24 00:12:19 UTC
I think I had the problem with 3.4.0 as well - that is probably why I installed 3.4.1rc1 ??

Will check it.
Comment 11 Petr Mladek 2011-06-24 03:36:42 UTC
Okay, we fixed this by reverting the commit 70af08e849a93d56915b7abe14537facf1022a6a and are going to do 3.4.1-rc3.
Comment 12 Michael Meeks 2011-06-24 03:57:09 UTC
*** Bug 38587 has been marked as a duplicate of this bug. ***
Comment 13 Jean-Baptiste Faure 2011-06-25 03:38:36 UTC
(In reply to comment #11)
> Okay, we fixed this by reverting the commit
> 70af08e849a93d56915b7abe14537facf1022a6a and are going to do 3.4.1-rc3.

It seems that the bug is still in the master.

Best regards. JBF
Comment 14 Jean-Baptiste Faure 2011-06-25 09:20:44 UTC
(In reply to comment #13)
> (In reply to comment #11)
> > Okay, we fixed this by reverting the commit
> > 70af08e849a93d56915b7abe14537facf1022a6a and are going to do 3.4.1-rc3.
> 
> It seems that the bug is still in the master.
> 
> Best regards. JBF

Ok, explications given by moggi on irc. Thanks.
Comment 15 Kohei Yoshida 2011-07-14 10:00:09 UTC
(In reply to comment #0)
> Created an attachment (id=48323) [details]
> spreadsheet with charts which cause probable memory leak
> 
> Steps to reproduce:
> - open the attached spreadsheet with LibO 3.4.1 rc1
> - go to the bottom of the sheet to show the charts
> - navigate in the sheet several times in order to show and hide the charts
> - after a moment it becomes impossible to save the file : too many files open !
> 
> In fact, if you look at the temporary folder used by LibO, you can see that
> Calc create many new temp files each time a chart becomes visible.
> With the attached file I reach quickly ~1000 files
> result of ulimit -n command : 1024 ...
> 
> Best regards. JBF

JBF,

Can you tell me

1) how many times (roughly) do I need to show and hide the charts in order to reproduce this?

2) where do you look to see the temp files that LibO creates, preferably on Linux?  Is it /tmp?

3) What are the names of those temp files?  Is there a pattern in the naming?

I'm trying to fix Bug 36688, and I'd like to make sure that I won't re-introduce this bug while fixing it.

Thanks,

Kohei
Comment 16 Kohei Yoshida 2011-07-14 11:22:18 UTC
Well, I did notice that I got tons of /tmp/lu41n115.tmp/lu41n*.tmp as I scrolled through the test document.  Each scroll-through would increase the number of temp files there.  With my (hopefully proper) fix for Bug 36688, I only see one temp file, and the number doesn't increase even after repeated scrolling, so I assume I've fixed Bug 36688 the right way.