Bug 64279 - REPORTBUILDER: Crash (segfault / invalid memory access) when opening report of attachment fdo#48056
Summary: REPORTBUILDER: Crash (segfault / invalid memory access) when opening report o...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
4.1.0.0.alpha0+ Master
Hardware: Other Linux (All)
: medium normal
Assignee: David Tardon
URL:
Whiteboard: target:4.2.0 target:4.1.0.0.beta2 tar...
Keywords:
: 65301 65594 65762 66083 66084 66225 66439 66510 66995 (view as bug list)
Depends on: 58371 61725 64150
Blocks: 58805 Database-Reports-Builder-MAB
  Show dependency treegraph
 
Reported: 2013-05-06 14:32 UTC by Lionel Elie Mamane
Modified: 2013-07-27 16:08 UTC (History)
21 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Lionel Elie Mamane 2013-05-06 14:32:32 UTC
+++ This bug was initially created as a clone of Bug #58371 +++

Problem description: 
On pc Debian x86-64 with master sources updated 2013-05-04 (commit 630bc850f7e36f75d295fc1d4a3cc56acd6921e7), trying to edit attachment 59233 [details] (from bug 48056) leads to a crash.

Steps to reproduce:
1. Open attachment 59233 [details] in LibreOffice
   (take care of calling it something.odb, not e.g. attachment.cgi?=id59233,
    due to a yet unreported bug in automatic content type sniffing in master)
2. In the left pane, click on "Reports"
3. In the lower right pane, right-click on "Bericht1"
4. In the pop-up contextual menu, click on "Edit"


Current behavior:
Crash

Expected behavior:
No crash and open the report in design mode


Analysis:

This comes from commit

commit 57082b1243e86694b72c5e4fad013bf207bfe81a
Author:     Luke Deller <luke@deller.id.au>
AuthorDate: Sun Apr 28 07:06:11 2013 +1000
Commit:     David Tardon <dtardon@redhat.com>
CommitDate: Sun Apr 28 11:52:12 2013 +0000

    fdo#60910: discard UNO shape object in SdrObject::SetPage


Reverting this commit fixes the crash. Why? Because during the parsing of the report document, when processing the end of a <text:p> tag:
 - SvXMLImport::endElement calls pContext->EndElement(), that is rptxml::OXMLFixedContent::EndElement
 - which calls rptxml::OXMLReportElementBase::EndElement()
 - which adds the corresponding reportdesign::OFixedText is added to its section
   (namely "Gruppenkopf", the group header):
   m_pContainer->getSection()->add(m_xComponent.get())
 - reportdesign::OSection::add itself adds it to the draw page: m_xDrawPage->add(xShape)
 - SvxDrawPage::add creates the corresponding sdr object, calling CreateSdrObject( xShape )
 - SvxDrawPage::CreateSdrObject, after creation, inserts the object into the page: mpPage->InsertObject(pObj)
 - mpPage is a rptui::OReportPage, its InsertObject member is SdrObjList::InsertObject,
 - which calls rptui::OReportPage::NbcInsertObject
 - which calls SdrObjList::NbcInsertObject
 - which calls SdrUnoObj::SetPage:  pObj->SetPage(pPage)
 - we thus end up (after some more virtual function calling its base class hierarchy fun)
   into SdrObject::SetPage
 - which this patch changed to setUnoShape(NULL), thereby depriving the rptui::OUnoObject
   from its UNO shape
 - continue execution, until back in rptui::OReportPage::NbcInsertObject
 - it calls pObj->getUnoShape(), which constructs a new UNO shape
   *but* this UNO shape is not referenced anywhere else than in m_xKeepShapeAlive
   (the meat happens in OObjectBase::getUnoShapeOf)

   this also gives an error/warning on stderr:
   "SvxShape::HasSdrObjectOwnership: have the ownership of an object which I don't know!"

 - rptui::OReportPage::NbcInsertObject ends with a call to pObjectBase->releaseUnoShape()
   which releases the m_xKeepShapeAlive reference, which leads to the UNO shape being
   deleted, which leads to trouble when subsequent code tries to use it.

By contrast, before this patch, the existing UNO shape is preserved, and all goes "well".

@dtardon, @luke.deller: where is the bug? In the above referenced commit or in reportdesign code? If the latter, what should change where in reportdesign code? Simply not call releaseUnoShape? Call some "setXXX" with the new shape?
Comment 1 Lionel Elie Mamane 2013-05-06 14:36:44 UTC
See attachment 78830 [details] for a backtrace.
Comment 2 Lionel Elie Mamane 2013-05-06 14:47:40 UTC
Also make sure to test in a version where bug 64150 is fixed/worked around. My work-around in master:


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

author	Lionel Elie Mamane <lionel@mamane.lu>	2013-05-03 15:42:12 (GMT)
committer	 Lionel Elie Mamane <lionel@mamane.lu>	2013-05-03 17:14:14 (GMT)
Comment 3 Lionel Elie Mamane 2013-05-12 03:07:22 UTC
Adding the developers that were in CC of the previous bugs of this saga: bug 58267, bug 60910.

This is related to the story of lifecycle of draw objects... Previous bugs were about calc and writer, but the latest patch in these bugs breaks reportbuilder charts.

In my local tree, I've just reverted http://cgit.freedesktop.org/libreoffice/core/commit/?id=57082b1243e86694b72c5e4fad013bf207bfe81a and it works well for me. I don't dare push that... Waiting for feedback.
Comment 4 Michael Meeks 2013-05-13 08:52:36 UTC
Urgh; another horrible side-effect of reverting:

commit bb3f2900a867fdcb6df916fff58199b4ce94dd05
Author: Michael Meeks <michael.meeks@suse.com>
Date:   Mon Dec 17 19:55:32 2012 +0000

    fdo#58399 - revert attempts to untangle and accelerate this mess.

:-) David ? you're the brave man with an ever deeper understanding of this heap - I'd like to think we're draining the swamp here - but wow; this was an expensive change !
Comment 5 David Tardon 2013-05-13 11:55:34 UTC
Without even looking around hard, I guess rptui::OCustomShape needs to override impl_setUnoShape. I meant to check this but have never got around to do it :-(
Comment 6 Commit Notification 2013-05-24 10:51:03 UTC
David Tardon committed a patch related to this issue.
It has been pushed to "master":

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

fdo#64279 do not crash opening report for editing



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 7 Michael Meeks 2013-05-24 12:34:15 UTC
Nice work David - I guess we really need to cleanly separate the semantics of adding shapes in the end :-) but presuambly that's best for 4.2 ;-)

Thanks !
Comment 8 Commit Notification 2013-05-24 14:28:40 UTC
David Tardon committed a patch related to this issue.
It has been pushed to "libreoffice-4-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ec2f66b51fe5a3874cbbbb21d4f386552cb05d9c&h=libreoffice-4-1

fdo#64279 do not crash opening report for editing


It will be available in LibreOffice 4.1.

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 9 Commit Notification 2013-05-28 15:20:42 UTC
David Tardon committed a patch related to this issue.
It has been pushed to "libreoffice-4-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7096efde95732b613dfa76df61a343b96ec6f979&h=libreoffice-4-0

fdo#64279 do not crash opening report for editing


It will be available in LibreOffice 4.0.5.

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 10 David Tardon 2013-06-04 09:58:26 UTC
*** Bug 65301 has been marked as a duplicate of this bug. ***
Comment 11 Michael Stahl (allotropia) 2013-06-20 21:28:11 UTC
*** Bug 65594 has been marked as a duplicate of this bug. ***
Comment 12 Lionel Elie Mamane 2013-06-21 04:48:27 UTC
*** Bug 65762 has been marked as a duplicate of this bug. ***
Comment 13 Robert Großkopf 2013-06-23 16:52:18 UTC
*** Bug 66084 has been marked as a duplicate of this bug. ***
Comment 14 Robert Großkopf 2013-06-23 16:55:03 UTC
*** Bug 66083 has been marked as a duplicate of this bug. ***
Comment 15 dany franck 2013-06-24 05:59:57 UTC
Nightly build 4.05: tested and OK.
Thanks
Comment 16 Julien Nabet 2013-06-26 18:41:23 UTC
According to Dany's comment, I put it VERIFIED.
Comment 17 Alex Thurgood 2013-06-27 10:42:38 UTC
*** Bug 66225 has been marked as a duplicate of this bug. ***
Comment 18 Michael Stahl (allotropia) 2013-07-01 13:01:14 UTC
*** Bug 66439 has been marked as a duplicate of this bug. ***
Comment 19 Robert Großkopf 2013-07-02 18:24:22 UTC
*** Bug 66510 has been marked as a duplicate of this bug. ***
Comment 20 Michael Stahl (allotropia) 2013-07-27 16:08:00 UTC
*** Bug 66995 has been marked as a duplicate of this bug. ***