[Kstars-devel] [kstars] kstars/skycomponents: Possible fix for bug #293183.

[Akarsh Simha]

Git commit 51c5f967e5954ef36682361889f252b49a8f9af3 by Akarsh Simha.
Committed on 05/02/2012 at 23:39.
Pushed by asimha into branch 'master'.

Possible fix for bug #293183.

The crash was caused by the following sequence of events:

* To reload constellation names / lines, the
  SkyMapComposite::reloadConstellationLines etc methods would delete
  the m_CNames and m_CLines instances and create new instances.

* In the process, the ConstellationNames etc constructors would emit
  progress text by way of SkyMapComposite::emitProgressText().

* This would display a splash screen (for too short a while to be seen
  most of the time. Maybe possible on slow computers). The splash
  screen seemed to call for a redraw of the sky map behind it.

* The redrawing of the sky map would access the m_CNames / m_CLines
  pointers and thereby cause a crash in many cases.

This crash sequence was verified by making the following change in
reloadConstellationLines and reloadConstellationNames:

* After delete m_CNames / m_CLines, add a m_CNames = 0 and m_CLines =
  0

The crash was reproducible _every time_ after this change (since the
pointers were now NULL and not pointing to an older block of memory)
and the backtrace after this change showed a call to a
ConstellationNames method with this = 0x0.

So this patch fixes the problem by making sure that we prevent the
drawing process while updating the constellation names / lines. The
code added (in the previous commit) also prevents recursive and
concurrent draws.

I'm not sure if this is the cleanest and the best solution. I'm
copying the mailing list to invite comments on this.

Will backport to 4.8

CCMAIL: kstars-devel at kde.org
BUG: 293183

M  +8    -0    kstars/skycomponents/skymapcomposite.cpp

http://commits.kde.org/kstars/51c5f967e5954ef36682361889f252b49a8f9af3

diff --git a/kstars/skycomponents/skymapcomposite.cpp b/kstars/skycomponents/skymapcomposite.cpp
index 42d1506..8422e10 100644
--- a/kstars/skycomponents/skymapcomposite.cpp
+++ b/kstars/skycomponents/skymapcomposite.cpp
@@ -473,14 +473,22 @@ void SkyMapComposite::removeCustomCatalog( const QString &name ) {
 }
 
 void SkyMapComposite::reloadCLines( ) {
+    Q_ASSERT( !SkyMapDrawAbstract::drawLock() );
+    SkyMapDrawAbstract::setDrawLock( true ); // This is not (yet) multithreaded, so I think we don't have to worry about overwriting the state of an existing lock --asimha
     delete m_CLines;
+    m_CLines = 0;
     m_CLines = new ConstellationLines( this, m_Cultures );
+    SkyMapDrawAbstract::setDrawLock( false );
 }
 
 void SkyMapComposite::reloadCNames( ) {
+    Q_ASSERT( !SkyMapDrawAbstract::drawLock() );
+    SkyMapDrawAbstract::setDrawLock( true ); // This is not (yet) multithreaded, so I think we don't have to worry about overwriting the state of an existing lock --asimha
     objectNames(SkyObject::CONSTELLATION).clear();
     delete m_CNames;
+    m_CNames = 0;
     m_CNames = new ConstellationNamesComponent( this, m_Cultures );
+    SkyMapDrawAbstract::setDrawLock( false );
 }
 
 bool SkyMapComposite::isLocalCNames() {