Commit 51c5f967 authored by Akarsh Simha's avatar Akarsh Simha
Browse files

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@kde.org
BUG: 293183
parent 861b663b
......@@ -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() {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment