SalomePlatform / gui

GUI module implements general user interface services of SALOME platform
GNU Lesser General Public License v2.1
1 stars 1 forks source link

QtxDockWidget creates segfault when closing Salome #1

Open GTchoumbia opened 2 weeks ago

GTchoumbia commented 2 weeks ago

Hi,

We've been upgrading our module to the latest Salome version (9.12.0) and this issue came up.

Basically, the destructor of QtxDockWidget is creating a segfault if the QtxDockWidget hasn't instantiated a Watcher, so when you use the two constructor that don't.

Two constructors of QtxDockWidget just initialize the Watcher object to 0, this one

QtxDockWidget::QtxDockWidget( const QString& title, QWidget* parent, Qt::WindowFlags f )
: QDockWidget( title, parent, f ),
  myWatcher( 0 ),
  myOrientation( (Qt::Orientation)-1 )

or this one, that we used to use in our codebase

QtxDockWidget::QtxDockWidget( QWidget* parent, Qt::WindowFlags f )
: QDockWidget( parent, f ),
myWatcher( 0 )

Which creates an undefined behaviour when calling the destructor

QtxDockWidget::~QtxDockWidget()
{
  myWatcher->setParent(nullptr);  //undefined behaviour if myWatcher = 0
  delete myWatcher;
  myWatcher = 0;
}

I'm guessing setParent is derenferencing the implicit this pointer at some point, which would explain the segfault.

Expectedly, it works fine if we use the constructor that instantiates a Watcher object.

QtxDockWidget::QtxDockWidget( const bool watch, QWidget* parent, Qt::WindowFlags f )
: QDockWidget( parent, f ),
  myWatcher( 0 ),
  myOrientation( (Qt::Orientation)-1 )
{
  if ( watch )
    myWatcher = new Watcher( this ); // myWatcher is not 0 anymore

  updateState();
}

Which we are currently using in our codebase to circumvent the issue, even if we have no need for a Watcher.

It also works fine as long as the QtxDockWidget has no parent, and the object is simply destroyed along with the process, but as soon as it has a parent, as far as I understand, Qt tries and destroy children automatically and calls the destructor along the way.

I suggest just adding a check for myWatcher, as seen in other parts of the file

QtxDockWidget::~QtxDockWidget()
{
    if (myWatcher)
    {
        myWatcher->setParent(nullptr); 
        delete myWatcher;
        myWatcher = 0;
    }
}

Hope it helps, Guy Tchoumbia.

nitawa commented 2 weeks ago

Please create a pull request following the steps described at this link .

nitawa commented 6 days ago

reference bos #42528