Bug Hunting Session
Bug 112848 - Assert when clicking on report wizard or design view (enable-dbgutil)
Summary: Assert when clicking on report wizard or design view (enable-dbgutil)
Status: CLOSED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
6.0.0.0.alpha0+
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Noel Grandin
URL:
Whiteboard: target:6.0.0
Keywords:
Depends on:
Blocks: Assert
  Show dependency treegraph
 
Reported: 2017-10-02 19:52 UTC by Julien Nabet
Modified: 2017-10-03 13:12 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
bt with debug symbols (14.81 KB, text/plain)
2017-10-02 19:53 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Nabet 2017-10-02 19:52:57 UTC
Description:
On pc Debian x86-64 with master sources updated today, I got an assert when clicking on report wizard.

Steps to Reproduce:
1. Create a brand new odb file (hsqldb embedded)
2. Click on Create report on design view or Use wizard to create report


Actual Results:  
Assertion

Expected Results:
No assertion


Reproducible: Always

User Profile Reset: No

Additional Info:


User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
Comment 1 Julien Nabet 2017-10-02 19:53:19 UTC
Created attachment 136711 [details]
bt with debug symbols
Comment 2 Julien Nabet 2017-10-02 19:55:23 UTC
Noel: thought you might be interested in this one after https://cgit.freedesktop.org/libreoffice/core/commit/?id=bc9a2ba677ce3fcd46c2bbef6e8faeacb14292c1
Comment 3 Julien Nabet 2017-10-02 20:20:53 UTC
Noel:
noticing there was a StartListening in SdrMarkView constructor, I tried to add one in destructor too:
diff --git a/svx/source/svdraw/svdmrkv.cxx b/svx/source/svdraw/svdmrkv.cxx
index 73a79d4bb983..ee1bf26c2a5b 100644
--- a/svx/source/svdraw/svdmrkv.cxx
+++ b/svx/source/svdraw/svdmrkv.cxx
@@ -158,6 +158,7 @@ SdrMarkView::SdrMarkView(SdrModel* pModel1, OutputDevice* pOut)
 
 SdrMarkView::~SdrMarkView()
 {
+    EndListening(*mpModel);
     // Migrate selections
     BrkMarkObj();
     BrkMarkPoints();

No better.

Then, I just commented the line in ReportSection.cxx:
diff --git a/reportdesign/source/ui/report/ReportSection.cxx b/reportdesign/source/ui/report/ReportSection.cxx
index 81eab1451313..91a624b6b9bf 100644
--- a/reportdesign/source/ui/report/ReportSection.cxx
+++ b/reportdesign/source/ui/report/ReportSection.cxx
@@ -229,7 +229,7 @@ void OReportSection::fill()
 
     m_pView->SetDesignMode();
 
-    m_pView->StartListening( *m_pModel  );
+    //m_pView->StartListening( *m_pModel  );
     m_pPage->SetSize( Size( getStyleProperty<awt::Size>(xReportDefinition,PROPERTY_PAPERSIZE).Width,5*m_xSection->getHeight()) );
     const Size aPageSize = m_pPage->GetSize();
     m_pView->SetWorkArea( tools::Rectangle( Point( nLeftMargin, 0), Size(aPageSize.Width() - nLeftMargin - nRightMargin,aPageSize.Height()) ) );

and finally got report wizard/design opened.

Since I don't know how these things work (even after having read the comments of your quoted commit), I haven't submitted them in gerrit.
Comment 4 Julien Nabet 2017-10-02 20:26:49 UTC
Taking a look to OReportSection::fill(), I noticed this:
m_pModel = m_pParent->getViewsWindow()->getView()->getReportView()->getController().getSdrModel();
(see https://opengrok.libreoffice.org/xref/core/reportdesign/source/ui/report/ReportSection.cxx#185)
but controller already uses StartListening:
   1737 bool OReportController::Construct(vcl::Window* pParent)
   1738 {
   1739     VclPtrInstance<ODesignView> pMyOwnView( pParent, m_xContext, *this );
   1740     StartListening( *pMyOwnView );
   1741     setView( pMyOwnView );
   1742 
   1743     // now that we have a view we can create the clipboard listener
   1744     m_aSystemClipboard = TransferableDataHelper::CreateFromSystemClipboard( getView() );
   1745     m_aSystemClipboard.StartClipboardListening( );
   1746     m_pClipboardNotifier = new TransferableClipboardListener( LINK( this, OReportController, OnClipboardChanged ) );
   1747     m_pClipboardNotifier->AddListener( getView() );
   1748 
   1749     OReportController_BASE::Construct(pParent);
   1750     return true;
   1751 }
(see https://opengrok.libreoffice.org/xref/core/reportdesign/source/ui/report/ReportController.cxx#1737)
Comment 5 Julien Nabet 2017-10-02 20:41:21 UTC
grepping call to StartListening in reportdesign and compare with EndListening, I noticed this too:
    117 OXUndoEnvironment::OXUndoEnvironment(OReportModel& _rModel)
    118                    :m_pImpl(new OXUndoEnvironmentImpl(_rModel) )
    119 {
    120     StartListening(m_pImpl->m_rModel);
    121 }
    122 
    123 
    124 OXUndoEnvironment::~OXUndoEnvironment()
    125 {
    126 }
(see https://opengrok.libreoffice.org/xref/core/reportdesign/source/core/sdr/UndoEnv.cxx#117)
shouldn't we add EndListening in destructor (to have a symetric behavior)?
Comment 6 Julien Nabet 2017-10-02 20:46:06 UTC
Forget my comment about removing too StartListening in ReportController my test was wrong, I can still select a table for the report
Comment 7 Commit Notification 2017-10-03 12:05:24 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

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

tdf#112848 Assert when clicking on report wizard or design view

It will be available in 6.0.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 Julien Nabet 2017-10-03 13:12:05 UTC
Noel: since it's an assert only, I suppose we won't backport it to 5.4 branch.
So let's put this one to FIXED.
Comment 9 Julien Nabet 2017-10-03 13:12:46 UTC
Verified since the patch corresponds to what I had suggested and so tested locally.