Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. Qt Development
  3. General and Desktop
  4. Crash when closing application

Crash when closing application

Scheduled Pinned Locked Moved Solved General and Desktop
crashrtti
22 Posts 3 Posters 10.6k Views
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • J Joel Bodenmann
    8 Jun 2016, 20:03

    @kshegunov
    Thank you for your reply.

    What I am getting is: SIGSEGV - Segmentation Fault. The debugger will jump to the destructor of the QWidget that was loaded from the shared library.
    Here's the stack trace:
    Stack Trace
    ToolWidget is my custom QWidget subclass that I display in a QDockWidget. That ToolWidget (which inherits from QObject) is loaded from the shared library. The CodeEditorPlugin contains an instance of the ToolWidget that it passes to the main application (in order to add it to the dock widget) by a simple getter:

    QWidget* CodeEditorPlugin::toolWidget()
    {
        return _toolWidget;
    }
    

    I am not deleting the widget myself. It is deleted automatically when my MainWindow gets deleted which is just when QApplication::exec() is about to return.

    There is no threading involved.

    K Offline
    K Offline
    kshegunov
    Moderators
    wrote on 8 Jun 2016, 20:11 last edited by kshegunov 6 Aug 2016, 20:17
    #4

    @Joel-Bodenmann
    One thing that jumps is that you apparently have two calls to your objects' destructors for some reason. How come the ToolWidget::~ToolWidget is called twice?

    As for the crash, if I had to guess, you have static QObject that's running de-init (the bottom of the stack) when the dll's unloading (but that's after QApplication's destructor has run, which isn't allowed). Can you confirm or deny this?

    /_dl_fini is the loader's unloading routine - meaning the dll's being unloaded at that point /

    Read and abide by the Qt Code of Conduct

    J 1 Reply Last reply 8 Jun 2016, 20:26
    2
    • K kshegunov
      8 Jun 2016, 20:11

      @Joel-Bodenmann
      One thing that jumps is that you apparently have two calls to your objects' destructors for some reason. How come the ToolWidget::~ToolWidget is called twice?

      As for the crash, if I had to guess, you have static QObject that's running de-init (the bottom of the stack) when the dll's unloading (but that's after QApplication's destructor has run, which isn't allowed). Can you confirm or deny this?

      /_dl_fini is the loader's unloading routine - meaning the dll's being unloaded at that point /

      J Offline
      J Offline
      Joel Bodenmann
      wrote on 8 Jun 2016, 20:26 last edited by Joel Bodenmann 6 Aug 2016, 20:33
      #5

      @kshegunov
      This is the first time I am working with plugins / shared libraries so I might need some more help, sorry for that.
      As far as I understand the documentation of QPluginLoader I never have to manually unload() or delete a plugin myself. According to the documentation the plugins will be automatically deleted when the application exits.
      My code is actually almost copy-paste from the example:

      unsigned PluginsManager::loadPlugins()
      {
          // Navigate to the plugins directory
          QDir pluginsDir(qApp->applicationDirPath());
      #if defined(Q_OS_WIN)
          if (pluginsDir.dirName().toLower() == "debug" || pluginsDir.dirName().toLower() == "release")
              pluginsDir.cdUp();
      #elif defined(Q_OS_MAC)
          if (pluginsDir.dirName() == "MacOS") {
              pluginsDir.cdUp();
              pluginsDir.cdUp();
              pluginsDir.cdUp();
          }
      #endif
          pluginsDir.cd("plugins");
      
          // Load each plugin
          for (const QString& fileName : pluginsDir.entryList(QDir::Files)) {
              QPluginLoader pluginLoader(pluginsDir.absoluteFilePath(fileName));
              QObject* pluginObject = pluginLoader.instance();
              if (pluginObject) {
                  // Check for plugins
                  Plugin* plugin = dynamic_cast<Plugin*>(pluginObject);
                  if (plugin) {
                      _pluginsModel->addPlugin(std::unique_ptr<Plugin>(plugin));
                  }
              }
          }
      
          return _pluginsModel->count();
      }
      

      Could the problem be that I am using unique pointers in my model? As seen in the code I basically transfer the owner-ship from the QPlugingLoader to my model class which maintains the plugins because the QPluginLoader gets out of scope anyway.
      Note that in my plugin model class I release the unique pointer and insert the raw pointer in a QList:

      bool PluginsModel::addPlugin(std::unique_ptr<Plugin> plugin)
      {
          // We take ownership - let's handle this without smart pointers because we can't put unique pointers in Qt containers
          Plugin* newPlugin = plugin.release();
      
          // Sanity check
          if (!newPlugin) {
              return false;
          }
      
          // Don't add the same thing multiple times
          if (_plugins.contains(newPlugin)) {
              return false;
          }
      
          // Add it
          _plugins.append(newPlugin);
      
          // Make this new plugin become the default viewer (if it's a viewer) when there's no default viewer yet
          if (dynamic_cast<Viewer*>(newPlugin)) {
              if (!_defaultViewerIndex.isValid()) {
                  _defaultViewerIndex = createIndex(_plugins.indexOf(newPlugin), 0, newPlugin);
              }
          }
      
          return true;
      }
      

      Note that I never call delete on the plugin myself.

      Regarding the fact that the destructor is getting called twice: I have no idea how or why this happens. I have to investigate. I definitely don't do it explicitly.

      Industrial process automation software: https://simulton.com
      Embedded Graphics & GUI library: https://ugfx.io

      K 1 Reply Last reply 8 Jun 2016, 20:34
      0
      • J Joel Bodenmann
        8 Jun 2016, 20:26

        @kshegunov
        This is the first time I am working with plugins / shared libraries so I might need some more help, sorry for that.
        As far as I understand the documentation of QPluginLoader I never have to manually unload() or delete a plugin myself. According to the documentation the plugins will be automatically deleted when the application exits.
        My code is actually almost copy-paste from the example:

        unsigned PluginsManager::loadPlugins()
        {
            // Navigate to the plugins directory
            QDir pluginsDir(qApp->applicationDirPath());
        #if defined(Q_OS_WIN)
            if (pluginsDir.dirName().toLower() == "debug" || pluginsDir.dirName().toLower() == "release")
                pluginsDir.cdUp();
        #elif defined(Q_OS_MAC)
            if (pluginsDir.dirName() == "MacOS") {
                pluginsDir.cdUp();
                pluginsDir.cdUp();
                pluginsDir.cdUp();
            }
        #endif
            pluginsDir.cd("plugins");
        
            // Load each plugin
            for (const QString& fileName : pluginsDir.entryList(QDir::Files)) {
                QPluginLoader pluginLoader(pluginsDir.absoluteFilePath(fileName));
                QObject* pluginObject = pluginLoader.instance();
                if (pluginObject) {
                    // Check for plugins
                    Plugin* plugin = dynamic_cast<Plugin*>(pluginObject);
                    if (plugin) {
                        _pluginsModel->addPlugin(std::unique_ptr<Plugin>(plugin));
                    }
                }
            }
        
            return _pluginsModel->count();
        }
        

        Could the problem be that I am using unique pointers in my model? As seen in the code I basically transfer the owner-ship from the QPlugingLoader to my model class which maintains the plugins because the QPluginLoader gets out of scope anyway.
        Note that in my plugin model class I release the unique pointer and insert the raw pointer in a QList:

        bool PluginsModel::addPlugin(std::unique_ptr<Plugin> plugin)
        {
            // We take ownership - let's handle this without smart pointers because we can't put unique pointers in Qt containers
            Plugin* newPlugin = plugin.release();
        
            // Sanity check
            if (!newPlugin) {
                return false;
            }
        
            // Don't add the same thing multiple times
            if (_plugins.contains(newPlugin)) {
                return false;
            }
        
            // Add it
            _plugins.append(newPlugin);
        
            // Make this new plugin become the default viewer (if it's a viewer) when there's no default viewer yet
            if (dynamic_cast<Viewer*>(newPlugin)) {
                if (!_defaultViewerIndex.isValid()) {
                    _defaultViewerIndex = createIndex(_plugins.indexOf(newPlugin), 0, newPlugin);
                }
            }
        
            return true;
        }
        

        Note that I never call delete on the plugin myself.

        Regarding the fact that the destructor is getting called twice: I have no idea how or why this happens. I have to investigate. I definitely don't do it explicitly.

        K Offline
        K Offline
        kshegunov
        Moderators
        wrote on 8 Jun 2016, 20:34 last edited by kshegunov 6 Aug 2016, 20:36
        #6

        @Joel-Bodenmann said:

        I never have to manually unload()

        Nope, you can call unload() and it can fail (if the plugin is tied to another loader).

        According to the documentation the plugins will be automatically deleted when the application exits.

        They are.

        std::unique_ptr<Plugin>(plugin)
        

        Don't do that. You don't own the pointer in the first place. Try to add one additional QPointer<Plugin> tracking that object to make sure the object is not deleted somewhere in the transferring. As a general rule, just use raw pointers (or guarded pointers to QObject - QPointer if you want to be extra careful).

        Could the problem be that I am using unique pointers in my model?

        I can't swear to it, but it's possible.

        Note that in my plugin model class I release the unique pointer and insert the raw pointer in a QList

        Why not do that directly when loading the plugins. I see no reason to do those shenanigans with the std::unique_ptr.

        PS.
        To be fully Qt compliant you should rather use qobject_cast instead of dynamic_cast. :)

        Read and abide by the Qt Code of Conduct

        J 1 Reply Last reply 8 Jun 2016, 20:43
        2
        • K kshegunov
          8 Jun 2016, 20:34

          @Joel-Bodenmann said:

          I never have to manually unload()

          Nope, you can call unload() and it can fail (if the plugin is tied to another loader).

          According to the documentation the plugins will be automatically deleted when the application exits.

          They are.

          std::unique_ptr<Plugin>(plugin)
          

          Don't do that. You don't own the pointer in the first place. Try to add one additional QPointer<Plugin> tracking that object to make sure the object is not deleted somewhere in the transferring. As a general rule, just use raw pointers (or guarded pointers to QObject - QPointer if you want to be extra careful).

          Could the problem be that I am using unique pointers in my model?

          I can't swear to it, but it's possible.

          Note that in my plugin model class I release the unique pointer and insert the raw pointer in a QList

          Why not do that directly when loading the plugins. I see no reason to do those shenanigans with the std::unique_ptr.

          PS.
          To be fully Qt compliant you should rather use qobject_cast instead of dynamic_cast. :)

          J Offline
          J Offline
          Joel Bodenmann
          wrote on 8 Jun 2016, 20:43 last edited by
          #7

          @kshegunov
          I changed it to use raw pointers everywhere. Sadly the problem persists. The stack trace looks exactly the same.
          I agree that the std::unique_ptr where pretty much a stupid decision at this point. Sorry for that :p

          I can't use qobject_cast because my Plugin is not inheriting from QObject:

          class Plugin
          {
          public:
              virtual ~Plugin() {}
          
              virtual QString pluginName() const = 0;
              virtual QString pluginVersion() const = 0;
              virtual QString pluginAuthor() const = 0;
              virtual QWidget* toolWidget() = 0;
              virtual QString toolWidgetName() const = 0;
              virtual QWidget* settingsWidget() = 0;
              virtual void settingsApply() = 0;
          };
          

          If I replace the dynamic_cast with the object_cast the plugin pointer becomes a nullpointer and therefore the plugin won't be available in my application. However, at least then my application doesn't crash when closing it :p

          Industrial process automation software: https://simulton.com
          Embedded Graphics & GUI library: https://ugfx.io

          K 1 Reply Last reply 8 Jun 2016, 20:46
          0
          • J Joel Bodenmann
            8 Jun 2016, 20:43

            @kshegunov
            I changed it to use raw pointers everywhere. Sadly the problem persists. The stack trace looks exactly the same.
            I agree that the std::unique_ptr where pretty much a stupid decision at this point. Sorry for that :p

            I can't use qobject_cast because my Plugin is not inheriting from QObject:

            class Plugin
            {
            public:
                virtual ~Plugin() {}
            
                virtual QString pluginName() const = 0;
                virtual QString pluginVersion() const = 0;
                virtual QString pluginAuthor() const = 0;
                virtual QWidget* toolWidget() = 0;
                virtual QString toolWidgetName() const = 0;
                virtual QWidget* settingsWidget() = 0;
                virtual void settingsApply() = 0;
            };
            

            If I replace the dynamic_cast with the object_cast the plugin pointer becomes a nullpointer and therefore the plugin won't be available in my application. However, at least then my application doesn't crash when closing it :p

            K Offline
            K Offline
            kshegunov
            Moderators
            wrote on 8 Jun 2016, 20:46 last edited by
            #8

            @Joel-Bodenmann

            Sorry for that :p

            Don't worry about it. Many are seduced by the STL and::its_beatiful*&api ;)

            I can't use qobject_cast because my Plugin is not inheriting from QObject

            This is of no consequence here. That's why moc was invented ;)
            You should have Q_DECLARE_INTERFACE somewhere? And I believe Q_INTERFACES is present in your implementation?
            This is enough to cast QObject * to your interface with qobject_cast.

            Read and abide by the Qt Code of Conduct

            J 1 Reply Last reply 8 Jun 2016, 20:55
            1
            • K kshegunov
              8 Jun 2016, 20:46

              @Joel-Bodenmann

              Sorry for that :p

              Don't worry about it. Many are seduced by the STL and::its_beatiful*&api ;)

              I can't use qobject_cast because my Plugin is not inheriting from QObject

              This is of no consequence here. That's why moc was invented ;)
              You should have Q_DECLARE_INTERFACE somewhere? And I believe Q_INTERFACES is present in your implementation?
              This is enough to cast QObject * to your interface with qobject_cast.

              J Offline
              J Offline
              Joel Bodenmann
              wrote on 8 Jun 2016, 20:55 last edited by Joel Bodenmann 6 Aug 2016, 21:04
              #9

              @kshegunov
              I actually have three plugin classes, each inheriting from the other:
              UML
              The corresponding headers:
              Plugin:

              #pragma once
              
              class QString;
              class QStringList;
              class QWidget;
              
              class Plugin
              {
              public:
                  virtual ~Plugin() {}
              
                  virtual QString pluginName() const = 0;
                  virtual QString pluginVersion() const = 0;
                  virtual QString pluginAuthor() const = 0;
                  virtual QWidget* toolWidget() = 0;
                  virtual QString toolWidgetName() const = 0;
                  virtual QWidget* settingsWidget() = 0;
                  virtual void settingsApply() = 0;
              };
              
              #define Plugin_iid "com.mycrashyapp.Plugin"
              Q_DECLARE_INTERFACE(Plugin, Plugin_iid)
              

              Viewer:

              #pragma once
              
              #include <memory>
              #include "interfaces/plugin.h"
              
              class QString;
              class QStringList;
              class QWidget;
              
              class Viewer : public Plugin
              {
              public:
                  virtual ~Viewer() {}
              
                  virtual std::unique_ptr<Viewer> clone() const = 0;  // Can't use copy-constructor in an interface
                  virtual QStringList fileExtensions() const = 0;
                  virtual bool fileOpen(const QString& filePath) = 0;
                  virtual void fileClose() = 0;
                  virtual bool fileExists() const = 0;
                  virtual QString fileName() const = 0;
                  virtual QString filePath() const = 0;
              };
              
              #define Viewer_iid "com.mycrashyapp.Viewer"
              Q_DECLARE_INTERFACE(Viewer, Viewer_iid)
              

              Editor:

              #pragma once
              
              #include "interfaces/viewer.h"
              
              class Editor : public Viewer
              {
              public:
                  virtual ~Editor() {}
              
                  virtual bool fileSave() = 0;
                  virtual bool fileSetPath(const QString& filePath) = 0;
                  virtual bool fileNeedsSave() const = 0;
              };
              
              #define Editor_iid "com.mycrashyapp.Editor"
              Q_DECLARE_INTERFACE(Editor, Editor_iid)
              

              Finally, this is how I create a plugin:

              #pragma once
              
              #include <QObject>
              #include "../elixpad/interfaces/editor.h"
              #include "settings/settings.h"
              
              class ToolWidget;
              class SettingsWidget;
              
              class CodeEditorPlugin : public QObject, public Editor
              {
                  Q_OBJECT
                  Q_PLUGIN_METADATA(IID Editor_iid)
                  Q_INTERFACES(Editor)
              
              public:
                  explicit CodeEditorPlugin(QObject* parent = nullptr);
                  virtual ~CodeEditorPlugin() override;
              
                  virtual std::unique_ptr<Viewer> clone() const override;
                  virtual QString pluginName() const override;
                  virtual QString pluginVersion() const override;
                  virtual QString pluginAuthor() const override;
                  virtual QString toolWidgetName() const override;
                  virtual QWidget* toolWidget() override;
                  virtual QWidget* settingsWidget() override;
                  virtual void settingsApply() override;
                  virtual QStringList fileExtensions() const override;
                  virtual bool fileOpen(const QString& filePath = QString()) override;
                  virtual void fileClose() override;
                  virtual bool fileExists() const override;
                  virtual QString fileName() const override;
                  virtual QString filePath() const override;
                  virtual bool fileSave() override;
                  virtual bool fileSetPath(const QString& filePath) override;
                  virtual bool fileNeedsSave() const override;
              
              private:
                  Q_DISABLE_COPY(CodeEditorPlugin)
              
                  ToolWidget* _toolWidget;
                  SettingsWidget* _settingsWidget;
              };
              

              I hope I didn't screw up too much... :S
              Using that code always returns a nullptr when using qobject_cast. Could this be related to my crash issues?

              And thank you very much for your help. I appreciate it a lot!

              P.S.: I don't really like the STL API ;)

              Industrial process automation software: https://simulton.com
              Embedded Graphics & GUI library: https://ugfx.io

              1 Reply Last reply
              0
              • K Offline
                K Offline
                kshegunov
                Moderators
                wrote on 8 Jun 2016, 21:08 last edited by kshegunov 6 Aug 2016, 21:08
                #10

                @Joel-Bodenmann said:

                I hope I didn't screw up too much... :S

                Nope, it looks pretty okay.

                 virtual std::unique_ptr<Viewer> clone() const override;
                

                Forget this stuff when working with QObjects they are non-copyable for a reason. You're only entering a world of hurt for no reason by exposing this.

                Using that code always returns a nullptr when using qobject_cast. Could this be related to my crash issues?

                It shouldn't but perhaps you should list all the interfaces:

                class CodeEditorPlugin : public QObject, public Editor
                {
                    Q_OBJECT
                    Q_PLUGIN_METADATA(IID Editor_iid)
                    Q_INTERFACES(Plugin Viewer Editor)
                    // ... 
                };
                

                And thank you very much for your help. I appreciate it a lot!

                You're welcome, let's hope I'll be able to help in the end. :)

                P.S.: I don't really like the STL API ;)

                It's so terrible that I'm pretty sure even its authors can't really stand it ... ;)


                May we take a look at how you create ToolWidget* _toolWidget; and where you pass it on to Qt (I assume you're doing), also if/where you delete the object?

                Read and abide by the Qt Code of Conduct

                J 1 Reply Last reply 8 Jun 2016, 21:20
                2
                • K kshegunov
                  8 Jun 2016, 21:08

                  @Joel-Bodenmann said:

                  I hope I didn't screw up too much... :S

                  Nope, it looks pretty okay.

                   virtual std::unique_ptr<Viewer> clone() const override;
                  

                  Forget this stuff when working with QObjects they are non-copyable for a reason. You're only entering a world of hurt for no reason by exposing this.

                  Using that code always returns a nullptr when using qobject_cast. Could this be related to my crash issues?

                  It shouldn't but perhaps you should list all the interfaces:

                  class CodeEditorPlugin : public QObject, public Editor
                  {
                      Q_OBJECT
                      Q_PLUGIN_METADATA(IID Editor_iid)
                      Q_INTERFACES(Plugin Viewer Editor)
                      // ... 
                  };
                  

                  And thank you very much for your help. I appreciate it a lot!

                  You're welcome, let's hope I'll be able to help in the end. :)

                  P.S.: I don't really like the STL API ;)

                  It's so terrible that I'm pretty sure even its authors can't really stand it ... ;)


                  May we take a look at how you create ToolWidget* _toolWidget; and where you pass it on to Qt (I assume you're doing), also if/where you delete the object?

                  J Offline
                  J Offline
                  Joel Bodenmann
                  wrote on 8 Jun 2016, 21:20 last edited by Joel Bodenmann 6 Aug 2016, 21:22
                  #11

                  @kshegunov said:

                  Forget this stuff when working with QObjects they are non-copyable for a reason. You're only entering a world of hurt for no reason by exposing this.

                  Sorry for annoying you with those unique pointers. I will get rid of them. I promise! :p
                  It just seems a good idea - in this case - as they explicitly transfer ownership of the cloned object to the caller.

                  It shouldn't but perhaps you should list all the interfaces:

                  Did that, no change, still crashing :(

                  It's so terrible that I'm pretty sure even its authors can't really stand it ... ;)

                  I thought I'm the only one... The API really is ridiculous in my opinion. Of course I understand that certain things are due to historical, legacy & consistency reasons but... such a terrible API :p

                  May we take a look at how you create ToolWidget* _toolWidget; and where you pass it on to Qt (I assume you're doing), also if/where you delete the object?

                  Yes Sir we may:

                  CodeEditorPlugin::CodeEditorPlugin(QObject* parent) : QObject(parent)
                  {
                      // Tool widget
                      _toolWidget = new ToolWidget;
                  
                      // Settings widget
                      _settingsWidget = new SettingsWidget;
                  }
                  
                  CodeEditorPlugin::~CodeEditorPlugin()
                  {
                      delete _toolWidget;
                      delete _settingsWidget;
                  }
                  
                  QWidget* CodeEditorPlugin::toolWidget()
                  {
                      return _toolWidget;
                  }
                  

                  And this is where I add the toolWidget to by Dock class which is a QDockWidget subclass:

                  Dock::Dock(QWidget* parent) : QDockWidget(parent)
                  {
                      ...
                      setAttribute(Qt::WA_DeleteOnClose, false);
                      ...
                  }
                  
                  bool Dock::setViewer(Viewer* viewer)
                  {
                      // Sanity check
                      if (!viewer) {
                          return false;
                      }
                  
                      // Make sure that the viewer has a tool widget
                      if (!viewer->toolWidget()) {
                          return false;
                      }
                  
                      setWidget(viewer->toolWidget());
                  
                      return true;
                  }
                  

                  And here's my cleanup / closing sequence:

                  bool MainWindow::fileClose(Viewer& viewer)
                  {
                      // First, save if it's an editor
                      Editor* editor = dynamic_cast<Editor*>(&viewer);
                      if (editor) {
                          if (!ViewerManager::viewerClose(*editor)) {
                              return false;
                          }
                      } else {
                          viewer.fileClose();
                      }
                  
                      // Get rid of the dock
                      Dock* dock = dockFromViewer(viewer);
                      if (dock) {
                          _docks.removeAll(dock);
                          delete dock;
                      }
                  
                      return true;
                  }
                  
                  bool MainWindow::fileCloseAll()
                  {
                      for (Viewer* viewer : viewers()) {
                          // Sanity check
                          if (!viewer) {
                              qCritical("MainWindow::fileCloseAll(): Invalid Viewer (nullptr).");
                              return false;
                          }
                  
                          // Close the viewer. Will prompt if user action required.
                          if (!fileClose(*viewer)) {
                              qInfo("MainWindow::fileCloseAll(): fileClose() failed.");
                              return false;
                          }
                      }
                  
                      return true;
                  }
                  
                  void MainWindow::closeEvent(QCloseEvent* event)
                  {
                      // Close all editors. Will prompt if save required.
                      if (!fileCloseAll()) {
                          event->ignore();
                          qInfo("MainWindow::closeEvent(): fileCloseAll() failed.");
                          return;
                      }
                  
                      // Save the state
                      QSettings settings;
                      settings.setValue("geometry", saveGeometry());
                      settings.setValue("windowState", saveState());
                  
                      // Close the window
                      QMainWindow::closeEvent(event);
                  }
                  

                  Note: I commented out both the delete in CodeEditorPlugin::~CodeEditorPlugin() on the _toolWidget and the delete in MainWindow::fileClose() on the dock. The crash remains.

                  Just for completeness, here's the relevant parts of the ViewerManager class that is used in MainWindow::fileClose() (both static methods):

                  bool ViewerManager::viewerClose(Viewer& viewer)
                  {
                      Editor* editor = dynamic_cast<Editor*>(&viewer);
                      if (editor) {
                          // Let the user decides if he wants to save or discard (or cancel)
                          if (editor->fileNeedsSave()) {
                              QMessageBox msgBox;
                              msgBox.setWindowIcon(Icons::favicon());
                              msgBox.setWindowTitle("Closing File");
                              msgBox.setText("The document has been modified.");
                              msgBox.setInformativeText("Do you want to save your changes?");
                              msgBox.setStandardButtons(QMessageBox::Save | QMessageBox::Discard | QMessageBox::Cancel);
                              msgBox.setDefaultButton(QMessageBox::Save);
                  
                              switch (msgBox.exec()) {
                              case QMessageBox::Save:
                                  if (!ViewerManager::editorSave(*editor)) {
                                      qInfo("ViewerManager::viewerClose(): ViewerManager::editorSave() failed.");
                                      return false;
                                  }
                                   break;
                  
                              case QMessageBox::Cancel:
                                  return false;
                  
                              case QMessageBox::Discard:
                                  break;
                  
                              default:
                                  break;
                              }
                          }
                      }
                  
                      // Ask the editor to close the file
                      viewer.fileClose();
                  
                      return true;
                  }
                  
                  bool ViewerManager::editorSave(Editor& editor)
                  {
                      // No reason to do anything if the file doesn't need to be saved
                      if (!editor.fileNeedsSave()) {
                          return true;
                      }
                  
                      // Ask the user for a file path if the file doesn't exist
                      if (!editor.fileExists()) {
                          // Get the path
                          QString filePath = QFileDialog::getSaveFileName(nullptr, "Save file as...");
                          if (filePath.isEmpty()) {
                              qInfo("ViewerManager::editorSave(): Invalid file name.");
                              return false;
                          }
                  
                          // Apply the path
                          if (!editor.fileSetPath(filePath)) {
                              qCritical("ViewerManager::editorSave(): Editor::fileSetPath() failed.");
                              return false;
                          }
                      }
                  
                      // Perform the actual saving action
                      if (!editor.fileSave()) {
                          qCritical("ViewerManager::editorSave(): Editor::fileSave() failed.");
                          return false;
                      }
                  
                      return true;
                  }
                  

                  Thank you for going through all this code. I hope that it's not too terrible to read.

                  Industrial process automation software: https://simulton.com
                  Embedded Graphics & GUI library: https://ugfx.io

                  K 1 Reply Last reply 8 Jun 2016, 21:35
                  0
                  • SGaistS Offline
                    SGaistS Offline
                    SGaist
                    Lifetime Qt Champion
                    wrote on 8 Jun 2016, 21:34 last edited by
                    #12

                    Hi,

                    Do I understand correctly that you have three plugins that are linked one to another ?

                    Interested in AI ? www.idiap.ch
                    Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

                    1 Reply Last reply
                    0
                    • J Joel Bodenmann
                      8 Jun 2016, 21:20

                      @kshegunov said:

                      Forget this stuff when working with QObjects they are non-copyable for a reason. You're only entering a world of hurt for no reason by exposing this.

                      Sorry for annoying you with those unique pointers. I will get rid of them. I promise! :p
                      It just seems a good idea - in this case - as they explicitly transfer ownership of the cloned object to the caller.

                      It shouldn't but perhaps you should list all the interfaces:

                      Did that, no change, still crashing :(

                      It's so terrible that I'm pretty sure even its authors can't really stand it ... ;)

                      I thought I'm the only one... The API really is ridiculous in my opinion. Of course I understand that certain things are due to historical, legacy & consistency reasons but... such a terrible API :p

                      May we take a look at how you create ToolWidget* _toolWidget; and where you pass it on to Qt (I assume you're doing), also if/where you delete the object?

                      Yes Sir we may:

                      CodeEditorPlugin::CodeEditorPlugin(QObject* parent) : QObject(parent)
                      {
                          // Tool widget
                          _toolWidget = new ToolWidget;
                      
                          // Settings widget
                          _settingsWidget = new SettingsWidget;
                      }
                      
                      CodeEditorPlugin::~CodeEditorPlugin()
                      {
                          delete _toolWidget;
                          delete _settingsWidget;
                      }
                      
                      QWidget* CodeEditorPlugin::toolWidget()
                      {
                          return _toolWidget;
                      }
                      

                      And this is where I add the toolWidget to by Dock class which is a QDockWidget subclass:

                      Dock::Dock(QWidget* parent) : QDockWidget(parent)
                      {
                          ...
                          setAttribute(Qt::WA_DeleteOnClose, false);
                          ...
                      }
                      
                      bool Dock::setViewer(Viewer* viewer)
                      {
                          // Sanity check
                          if (!viewer) {
                              return false;
                          }
                      
                          // Make sure that the viewer has a tool widget
                          if (!viewer->toolWidget()) {
                              return false;
                          }
                      
                          setWidget(viewer->toolWidget());
                      
                          return true;
                      }
                      

                      And here's my cleanup / closing sequence:

                      bool MainWindow::fileClose(Viewer& viewer)
                      {
                          // First, save if it's an editor
                          Editor* editor = dynamic_cast<Editor*>(&viewer);
                          if (editor) {
                              if (!ViewerManager::viewerClose(*editor)) {
                                  return false;
                              }
                          } else {
                              viewer.fileClose();
                          }
                      
                          // Get rid of the dock
                          Dock* dock = dockFromViewer(viewer);
                          if (dock) {
                              _docks.removeAll(dock);
                              delete dock;
                          }
                      
                          return true;
                      }
                      
                      bool MainWindow::fileCloseAll()
                      {
                          for (Viewer* viewer : viewers()) {
                              // Sanity check
                              if (!viewer) {
                                  qCritical("MainWindow::fileCloseAll(): Invalid Viewer (nullptr).");
                                  return false;
                              }
                      
                              // Close the viewer. Will prompt if user action required.
                              if (!fileClose(*viewer)) {
                                  qInfo("MainWindow::fileCloseAll(): fileClose() failed.");
                                  return false;
                              }
                          }
                      
                          return true;
                      }
                      
                      void MainWindow::closeEvent(QCloseEvent* event)
                      {
                          // Close all editors. Will prompt if save required.
                          if (!fileCloseAll()) {
                              event->ignore();
                              qInfo("MainWindow::closeEvent(): fileCloseAll() failed.");
                              return;
                          }
                      
                          // Save the state
                          QSettings settings;
                          settings.setValue("geometry", saveGeometry());
                          settings.setValue("windowState", saveState());
                      
                          // Close the window
                          QMainWindow::closeEvent(event);
                      }
                      

                      Note: I commented out both the delete in CodeEditorPlugin::~CodeEditorPlugin() on the _toolWidget and the delete in MainWindow::fileClose() on the dock. The crash remains.

                      Just for completeness, here's the relevant parts of the ViewerManager class that is used in MainWindow::fileClose() (both static methods):

                      bool ViewerManager::viewerClose(Viewer& viewer)
                      {
                          Editor* editor = dynamic_cast<Editor*>(&viewer);
                          if (editor) {
                              // Let the user decides if he wants to save or discard (or cancel)
                              if (editor->fileNeedsSave()) {
                                  QMessageBox msgBox;
                                  msgBox.setWindowIcon(Icons::favicon());
                                  msgBox.setWindowTitle("Closing File");
                                  msgBox.setText("The document has been modified.");
                                  msgBox.setInformativeText("Do you want to save your changes?");
                                  msgBox.setStandardButtons(QMessageBox::Save | QMessageBox::Discard | QMessageBox::Cancel);
                                  msgBox.setDefaultButton(QMessageBox::Save);
                      
                                  switch (msgBox.exec()) {
                                  case QMessageBox::Save:
                                      if (!ViewerManager::editorSave(*editor)) {
                                          qInfo("ViewerManager::viewerClose(): ViewerManager::editorSave() failed.");
                                          return false;
                                      }
                                       break;
                      
                                  case QMessageBox::Cancel:
                                      return false;
                      
                                  case QMessageBox::Discard:
                                      break;
                      
                                  default:
                                      break;
                                  }
                              }
                          }
                      
                          // Ask the editor to close the file
                          viewer.fileClose();
                      
                          return true;
                      }
                      
                      bool ViewerManager::editorSave(Editor& editor)
                      {
                          // No reason to do anything if the file doesn't need to be saved
                          if (!editor.fileNeedsSave()) {
                              return true;
                          }
                      
                          // Ask the user for a file path if the file doesn't exist
                          if (!editor.fileExists()) {
                              // Get the path
                              QString filePath = QFileDialog::getSaveFileName(nullptr, "Save file as...");
                              if (filePath.isEmpty()) {
                                  qInfo("ViewerManager::editorSave(): Invalid file name.");
                                  return false;
                              }
                      
                              // Apply the path
                              if (!editor.fileSetPath(filePath)) {
                                  qCritical("ViewerManager::editorSave(): Editor::fileSetPath() failed.");
                                  return false;
                              }
                          }
                      
                          // Perform the actual saving action
                          if (!editor.fileSave()) {
                              qCritical("ViewerManager::editorSave(): Editor::fileSave() failed.");
                              return false;
                          }
                      
                          return true;
                      }
                      

                      Thank you for going through all this code. I hope that it's not too terrible to read.

                      K Offline
                      K Offline
                      kshegunov
                      Moderators
                      wrote on 8 Jun 2016, 21:35 last edited by kshegunov 6 Aug 2016, 21:37
                      #13

                      @Joel-Bodenmann said:

                      Sorry for annoying you with those unique pointers. I will get rid of them. I promise! :p

                      I actually meant the cloning. You shouldn't clone QObjects at all. And since this is a plugin interface, it's a pretty safe bet the implementation would be a QObject subclass. :)

                      It shouldn't but perhaps you should list all the interfaces:

                      Did that, no change, still crashing :(

                      I meant to make qobject_cast work, not to solve the crash.

                      The API really is ridiculous in my opinion.

                      Couldn't have put it better myself.


                      Right. I think you may be getting a double delete somehow (although your PS makes that less probable). Still this:

                       setWidget(viewer->toolWidget());
                      

                      Will take ownership of the passed widget.

                      Could you try the following. Change the raw pointers in the plugin with QPointer<> specializations:

                      QPointer<ToolWidget> _toolWidget;
                      QPointer<SettingsWidget> _settingsWidget;
                      

                      Then in the destructor you can do:

                      CodeEditorPlugin::~CodeEditorPlugin()
                      {
                          if (_toolWidget)
                              delete _toolWidget;
                      
                          if (_settingsWidget)
                              delete _settingsWidget;
                      }
                      

                      Also there two:

                      setAttribute(Qt::WA_DeleteOnClose, false);
                      

                      and

                       delete dock; // MainWindow::fileClose
                      

                      Look somewhat suspicious. (EDIT: I missed that you actually disable the delete on close attribute)
                      But then you said you removed the delete. Perhaps you could try stripping most of the code to prepare a minimal example that reproduces this? I can't find anything plainly wrong besides the two remarks above.

                      Read and abide by the Qt Code of Conduct

                      J 1 Reply Last reply 9 Jun 2016, 05:24
                      2
                      • K kshegunov
                        8 Jun 2016, 21:35

                        @Joel-Bodenmann said:

                        Sorry for annoying you with those unique pointers. I will get rid of them. I promise! :p

                        I actually meant the cloning. You shouldn't clone QObjects at all. And since this is a plugin interface, it's a pretty safe bet the implementation would be a QObject subclass. :)

                        It shouldn't but perhaps you should list all the interfaces:

                        Did that, no change, still crashing :(

                        I meant to make qobject_cast work, not to solve the crash.

                        The API really is ridiculous in my opinion.

                        Couldn't have put it better myself.


                        Right. I think you may be getting a double delete somehow (although your PS makes that less probable). Still this:

                         setWidget(viewer->toolWidget());
                        

                        Will take ownership of the passed widget.

                        Could you try the following. Change the raw pointers in the plugin with QPointer<> specializations:

                        QPointer<ToolWidget> _toolWidget;
                        QPointer<SettingsWidget> _settingsWidget;
                        

                        Then in the destructor you can do:

                        CodeEditorPlugin::~CodeEditorPlugin()
                        {
                            if (_toolWidget)
                                delete _toolWidget;
                        
                            if (_settingsWidget)
                                delete _settingsWidget;
                        }
                        

                        Also there two:

                        setAttribute(Qt::WA_DeleteOnClose, false);
                        

                        and

                         delete dock; // MainWindow::fileClose
                        

                        Look somewhat suspicious. (EDIT: I missed that you actually disable the delete on close attribute)
                        But then you said you removed the delete. Perhaps you could try stripping most of the code to prepare a minimal example that reproduces this? I can't find anything plainly wrong besides the two remarks above.

                        J Offline
                        J Offline
                        Joel Bodenmann
                        wrote on 9 Jun 2016, 05:24 last edited by Joel Bodenmann 6 Sept 2016, 06:00
                        #14

                        @SGaist

                        Do I understand correctly that you have three plugins that are linked one to another ?

                        Yes I am aware of that. In fact, they are not just somehow "linked" together but they actually inherit from each other. This is the behavior that I want. I want to be able to load plugins in my application that allow just viewing a file without modifying it and plugins that are full editors that allow modifying and saving the file. As you can see in the UML diagram from my previous post the Editor needs everything the Viewer provides.
                        The Plugin class is just a base class that allows me retrieving basic information about the plugin such as the plugin name, version and the author. This base class will also be useful in the future when there are plugins that aren't actually file viewers.
                        To summarize: Viewer inherits form Plugin, Editor inherits from Viewer.

                        @kshegunov said:

                        Sorry for annoying you with those unique pointers. I will get rid of them. I promise! :p
                        I actually meant the cloning. You shouldn't clone QObjects at all. And since this is a plugin interface, it's a pretty safe bet the implementation would be a QObject subclass. :)

                        I need a way to clone my Viewer. As you can see, the Viewer is a plugin that allows opening and displaying a file. The Viewer::toolWidget() is the actual widget that displays the file that will be added to the dock widget. As I want to be able to open multiple files at once, I need a way to create multiple instances of the same Viewer class/plugin.
                        But when actually writing this... I guess the name clone() is misleading in this case. Because all it does is creating a new object and returning it directly:

                        std::unique_ptr<Viewer> CodeEditorPlugin::clone() const
                        {
                            return std::unique_ptr<Viewer>(new CodeEditorPlugin);
                        }
                        

                        I actually meant the cloning. You shouldn't clone QObjects at all. And since this is a plugin interface, it's a pretty safe bet the implementation would be a QObject subclass. :)

                        Oh... Sorry... Yeah now the qobject_cast is actually working. I guess I have some more reading to do before I understand why. But thanks for the remark! I like to be Qt compliant wherever possible ;)

                        Could you try the following. Change the raw pointers in the plugin with QPointer<> specializations:

                        Did that, same result. The issue persists.

                        I will try to create a minimum test-case that allows you reproducing the problem. However, that might take a couple of days (most likely I'll get it done over the weekend).
                        I just hope that I am able to reproduce the error in a stripped down version :p

                        Industrial process automation software: https://simulton.com
                        Embedded Graphics & GUI library: https://ugfx.io

                        K 1 Reply Last reply 9 Jun 2016, 10:41
                        0
                        • SGaistS Offline
                          SGaistS Offline
                          SGaist
                          Lifetime Qt Champion
                          wrote on 9 Jun 2016, 07:39 last edited by
                          #15

                          Just to be sure, you have something like:

                          plugin2.pro:

                          LIBS += -lplugin1
                          

                          plugin3.pro:

                          LIBS += -lplugin2
                          

                          ?

                          Interested in AI ? www.idiap.ch
                          Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

                          J 1 Reply Last reply 9 Jun 2016, 08:41
                          0
                          • SGaistS SGaist
                            9 Jun 2016, 07:39

                            Just to be sure, you have something like:

                            plugin2.pro:

                            LIBS += -lplugin1
                            

                            plugin3.pro:

                            LIBS += -lplugin2
                            

                            ?

                            J Offline
                            J Offline
                            Joel Bodenmann
                            wrote on 9 Jun 2016, 08:41 last edited by
                            #16

                            @SGaist
                            Not at all. The Plugin, Viewer and Editor files are all single header files in my main application directory. My plugins (eg. the CodeEditor) are separate projects that simply include those files:

                            elixpad.pro and plugins.pro are just project files containing the sub-projects. Nothing in there besides SUBDIRS.

                            This is what the plugin project files look like (codeeditorplugin.pro):

                            TEMPLATE        = lib
                            CONFIG         += plugin silent c++11
                            QT             += widgets
                            INCLUDEPATH    += ../../elixpad
                            TARGET          = $$qtLibraryTarget(codeeditorplugin)
                            DESTDIR         = ../../elixpad/plugins
                            

                            @SGaist Just to be sure that we are on the same page: Everything works great. I can use the plugins and I don't have any issues at all. The only issue I am experiencing is the crash when the application closes.

                            Industrial process automation software: https://simulton.com
                            Embedded Graphics & GUI library: https://ugfx.io

                            1 Reply Last reply
                            0
                            • J Joel Bodenmann
                              9 Jun 2016, 05:24

                              @SGaist

                              Do I understand correctly that you have three plugins that are linked one to another ?

                              Yes I am aware of that. In fact, they are not just somehow "linked" together but they actually inherit from each other. This is the behavior that I want. I want to be able to load plugins in my application that allow just viewing a file without modifying it and plugins that are full editors that allow modifying and saving the file. As you can see in the UML diagram from my previous post the Editor needs everything the Viewer provides.
                              The Plugin class is just a base class that allows me retrieving basic information about the plugin such as the plugin name, version and the author. This base class will also be useful in the future when there are plugins that aren't actually file viewers.
                              To summarize: Viewer inherits form Plugin, Editor inherits from Viewer.

                              @kshegunov said:

                              Sorry for annoying you with those unique pointers. I will get rid of them. I promise! :p
                              I actually meant the cloning. You shouldn't clone QObjects at all. And since this is a plugin interface, it's a pretty safe bet the implementation would be a QObject subclass. :)

                              I need a way to clone my Viewer. As you can see, the Viewer is a plugin that allows opening and displaying a file. The Viewer::toolWidget() is the actual widget that displays the file that will be added to the dock widget. As I want to be able to open multiple files at once, I need a way to create multiple instances of the same Viewer class/plugin.
                              But when actually writing this... I guess the name clone() is misleading in this case. Because all it does is creating a new object and returning it directly:

                              std::unique_ptr<Viewer> CodeEditorPlugin::clone() const
                              {
                                  return std::unique_ptr<Viewer>(new CodeEditorPlugin);
                              }
                              

                              I actually meant the cloning. You shouldn't clone QObjects at all. And since this is a plugin interface, it's a pretty safe bet the implementation would be a QObject subclass. :)

                              Oh... Sorry... Yeah now the qobject_cast is actually working. I guess I have some more reading to do before I understand why. But thanks for the remark! I like to be Qt compliant wherever possible ;)

                              Could you try the following. Change the raw pointers in the plugin with QPointer<> specializations:

                              Did that, same result. The issue persists.

                              I will try to create a minimum test-case that allows you reproducing the problem. However, that might take a couple of days (most likely I'll get it done over the weekend).
                              I just hope that I am able to reproduce the error in a stripped down version :p

                              K Offline
                              K Offline
                              kshegunov
                              Moderators
                              wrote on 9 Jun 2016, 10:41 last edited by
                              #17

                              @Joel-Bodenmann said:

                              The Plugin class is just a base class that allows me retrieving basic information about the plugin such as the plugin name, version and the author. This base class will also be useful in the future when there are plugins that aren't actually file viewers.
                              To summarize: Viewer inherits form Plugin, Editor inherits from Viewer.

                              I wouldn't design it exactly like this, but I suggest leaving that question for after finding the problem.

                              But when actually writing this... I guess the name clone() is misleading in this case.

                              I must have been misled. I thought you're copying QObjects with this.

                              As you can see, the Viewer is a plugin that allows opening and displaying a file.

                              I don't think it should be. But rather a class that's exposed from the plugin. That's what I was talking about in the first sentence. A plugin is a single(ton) object (of the .dll/.so) that represents the entry point. You can think of it as a factory of sorts. So usually the most convenient way to deal with that is like this:

                              class ViewerProvider
                              {
                              public:
                                  virtual QList<Viewer> viewers() = 0;
                              }
                              Q_DECLARE_INTERFACE(ViewerProvider) //< A plugin interface
                              
                              class EditorProvider
                              {
                              public:
                                  virtual QList<Editor> editors() = 0;
                              };
                              Q_DECLARE_INTERFACE(EditorProvider) //< A plugin interface
                              

                              Then the plugin will implement both if it provides both editors and viewers:

                              class MyCoolPlugin : public QObject, public ViewerProvider, public EditorProvider
                              {
                                  Q_OBJECT
                                  Q_PLUGIN_METADATA(...)
                                  Q_INTERFACES(ViewerProvider EditorProvider) //< Listing all interfaces so moc will generate the TI we need for qobject_cast
                                  // ....
                              };
                              

                              Now, when loading we can inquire the plugin what it provides:

                              QPluginLoader pluginLoader(...);
                              QObject * pluginObject = pluginLoader.instance(); //< This is the plugin - the entry point of the library, not the functionality we're after
                              EditorProvider * editorprovider = qobject_cast<EditorProvider>(pluginObject);
                              if (editorprovider)  {
                                 // Superb our plugin provides editors, we can load those, or store the pointer to the plugin or whatever we need to do
                                 // The point is the plugin itself only aggregates the functionality
                              }
                              // ... And so on (analogically with ViewerProvider)
                              

                              And by the way you don't need the destructors in your interfaces. They will generate object code (because virtual methods can't be inlined) and you don't really need to enforce them virtual, as QObject already does that.

                              I will try to create a minimum test-case that allows you reproducing the problem. However, that might take a couple of days (most likely I'll get it done over the weekend).

                              If there's nothing secretive about the code (i.e. it's not protected by copyright or something like this) you could upload it in a repo somewhere as it is. I'd download it in the evening and check if I can find the problem.

                              Kind regards.

                              Read and abide by the Qt Code of Conduct

                              J 2 Replies Last reply 9 Jun 2016, 13:56
                              1
                              • K kshegunov
                                9 Jun 2016, 10:41

                                @Joel-Bodenmann said:

                                The Plugin class is just a base class that allows me retrieving basic information about the plugin such as the plugin name, version and the author. This base class will also be useful in the future when there are plugins that aren't actually file viewers.
                                To summarize: Viewer inherits form Plugin, Editor inherits from Viewer.

                                I wouldn't design it exactly like this, but I suggest leaving that question for after finding the problem.

                                But when actually writing this... I guess the name clone() is misleading in this case.

                                I must have been misled. I thought you're copying QObjects with this.

                                As you can see, the Viewer is a plugin that allows opening and displaying a file.

                                I don't think it should be. But rather a class that's exposed from the plugin. That's what I was talking about in the first sentence. A plugin is a single(ton) object (of the .dll/.so) that represents the entry point. You can think of it as a factory of sorts. So usually the most convenient way to deal with that is like this:

                                class ViewerProvider
                                {
                                public:
                                    virtual QList<Viewer> viewers() = 0;
                                }
                                Q_DECLARE_INTERFACE(ViewerProvider) //< A plugin interface
                                
                                class EditorProvider
                                {
                                public:
                                    virtual QList<Editor> editors() = 0;
                                };
                                Q_DECLARE_INTERFACE(EditorProvider) //< A plugin interface
                                

                                Then the plugin will implement both if it provides both editors and viewers:

                                class MyCoolPlugin : public QObject, public ViewerProvider, public EditorProvider
                                {
                                    Q_OBJECT
                                    Q_PLUGIN_METADATA(...)
                                    Q_INTERFACES(ViewerProvider EditorProvider) //< Listing all interfaces so moc will generate the TI we need for qobject_cast
                                    // ....
                                };
                                

                                Now, when loading we can inquire the plugin what it provides:

                                QPluginLoader pluginLoader(...);
                                QObject * pluginObject = pluginLoader.instance(); //< This is the plugin - the entry point of the library, not the functionality we're after
                                EditorProvider * editorprovider = qobject_cast<EditorProvider>(pluginObject);
                                if (editorprovider)  {
                                   // Superb our plugin provides editors, we can load those, or store the pointer to the plugin or whatever we need to do
                                   // The point is the plugin itself only aggregates the functionality
                                }
                                // ... And so on (analogically with ViewerProvider)
                                

                                And by the way you don't need the destructors in your interfaces. They will generate object code (because virtual methods can't be inlined) and you don't really need to enforce them virtual, as QObject already does that.

                                I will try to create a minimum test-case that allows you reproducing the problem. However, that might take a couple of days (most likely I'll get it done over the weekend).

                                If there's nothing secretive about the code (i.e. it's not protected by copyright or something like this) you could upload it in a repo somewhere as it is. I'd download it in the evening and check if I can find the problem.

                                Kind regards.

                                J Offline
                                J Offline
                                Joel Bodenmann
                                wrote on 9 Jun 2016, 13:56 last edited by
                                #18

                                @kshegunov
                                Thank you for your comments regarding the design choices. I will be happy to talk more about this once the current issue is resolved. I am very keen on learning how to do it the right / better way. It's the first time I am working with plugins and the likelihood of getting it wrong is quite high with these sorts of things.

                                I assume that the poor design choices I made are not the issue that is leading to the current problem?

                                I'll make sure that you get access to the code base. Thank you once again for your help!

                                Industrial process automation software: https://simulton.com
                                Embedded Graphics & GUI library: https://ugfx.io

                                1 Reply Last reply
                                0
                                • K Offline
                                  K Offline
                                  kshegunov
                                  Moderators
                                  wrote on 9 Jun 2016, 17:14 last edited by kshegunov 6 Sept 2016, 17:15
                                  #19

                                  @Joel-Bodenmann
                                  I found the error. The problem is you have widgets that outlive your QApplication object. So calling delete on them (after the root QObject has died) makes all kinds of nasty stuff inside Qt. You should clearly(!) declare your objects' lifetimes.

                                  That singleton Preferences object is holding references to QObject instances which will be deleted after the main()'s stack frame has closed - not cool.
                                  If you remove the delete statements in your plugin destructors you will not get a crash, but memory is leaked (not crucial here, but a good thing to fix).

                                  The interesting part is that the exact same binary crashes on Windows 7, but not on Windows 10. I also get the crash on Ubuntu 15.04.

                                  That's loader init/deinit for you. Or to cite myself:

                                  As for the crash, if I had to guess, you have static QObject that's running de-init (the bottom of the stack) when the dll's unloading (but that's after QApplication's destructor has run, which isn't allowed).

                                  Not an exact match, but close enough. You can't have widgets (or QObjects in general) outside of QApplication's lifetime. /there are few minor exceptions, which have no bearing here/ :)


                                  If you ask for a "good" fix - I recommend reconsidering your design.
                                  Also, use the designer(!), otherwise you'd end up only writing widget positioning/tweaking code and nothing else. :)

                                  Kind regards.

                                  Read and abide by the Qt Code of Conduct

                                  J 1 Reply Last reply 19 Jun 2016, 15:38
                                  2
                                  • K kshegunov
                                    9 Jun 2016, 17:14

                                    @Joel-Bodenmann
                                    I found the error. The problem is you have widgets that outlive your QApplication object. So calling delete on them (after the root QObject has died) makes all kinds of nasty stuff inside Qt. You should clearly(!) declare your objects' lifetimes.

                                    That singleton Preferences object is holding references to QObject instances which will be deleted after the main()'s stack frame has closed - not cool.
                                    If you remove the delete statements in your plugin destructors you will not get a crash, but memory is leaked (not crucial here, but a good thing to fix).

                                    The interesting part is that the exact same binary crashes on Windows 7, but not on Windows 10. I also get the crash on Ubuntu 15.04.

                                    That's loader init/deinit for you. Or to cite myself:

                                    As for the crash, if I had to guess, you have static QObject that's running de-init (the bottom of the stack) when the dll's unloading (but that's after QApplication's destructor has run, which isn't allowed).

                                    Not an exact match, but close enough. You can't have widgets (or QObjects in general) outside of QApplication's lifetime. /there are few minor exceptions, which have no bearing here/ :)


                                    If you ask for a "good" fix - I recommend reconsidering your design.
                                    Also, use the designer(!), otherwise you'd end up only writing widget positioning/tweaking code and nothing else. :)

                                    Kind regards.

                                    J Offline
                                    J Offline
                                    Joel Bodenmann
                                    wrote on 19 Jun 2016, 15:38 last edited by Joel Bodenmann
                                    #20

                                    So to wrap this up: The problem is/was that QPluginLoader::unload() is automatically called AFTER the application itself has been destroyed. Hence one shouldn't destroy the plugin objects before by either manually calling delete on them or by setting a parent that will be deleted.

                                    Industrial process automation software: https://simulton.com
                                    Embedded Graphics & GUI library: https://ugfx.io

                                    1 Reply Last reply
                                    0
                                    • K kshegunov
                                      9 Jun 2016, 10:41

                                      @Joel-Bodenmann said:

                                      The Plugin class is just a base class that allows me retrieving basic information about the plugin such as the plugin name, version and the author. This base class will also be useful in the future when there are plugins that aren't actually file viewers.
                                      To summarize: Viewer inherits form Plugin, Editor inherits from Viewer.

                                      I wouldn't design it exactly like this, but I suggest leaving that question for after finding the problem.

                                      But when actually writing this... I guess the name clone() is misleading in this case.

                                      I must have been misled. I thought you're copying QObjects with this.

                                      As you can see, the Viewer is a plugin that allows opening and displaying a file.

                                      I don't think it should be. But rather a class that's exposed from the plugin. That's what I was talking about in the first sentence. A plugin is a single(ton) object (of the .dll/.so) that represents the entry point. You can think of it as a factory of sorts. So usually the most convenient way to deal with that is like this:

                                      class ViewerProvider
                                      {
                                      public:
                                          virtual QList<Viewer> viewers() = 0;
                                      }
                                      Q_DECLARE_INTERFACE(ViewerProvider) //< A plugin interface
                                      
                                      class EditorProvider
                                      {
                                      public:
                                          virtual QList<Editor> editors() = 0;
                                      };
                                      Q_DECLARE_INTERFACE(EditorProvider) //< A plugin interface
                                      

                                      Then the plugin will implement both if it provides both editors and viewers:

                                      class MyCoolPlugin : public QObject, public ViewerProvider, public EditorProvider
                                      {
                                          Q_OBJECT
                                          Q_PLUGIN_METADATA(...)
                                          Q_INTERFACES(ViewerProvider EditorProvider) //< Listing all interfaces so moc will generate the TI we need for qobject_cast
                                          // ....
                                      };
                                      

                                      Now, when loading we can inquire the plugin what it provides:

                                      QPluginLoader pluginLoader(...);
                                      QObject * pluginObject = pluginLoader.instance(); //< This is the plugin - the entry point of the library, not the functionality we're after
                                      EditorProvider * editorprovider = qobject_cast<EditorProvider>(pluginObject);
                                      if (editorprovider)  {
                                         // Superb our plugin provides editors, we can load those, or store the pointer to the plugin or whatever we need to do
                                         // The point is the plugin itself only aggregates the functionality
                                      }
                                      // ... And so on (analogically with ViewerProvider)
                                      

                                      And by the way you don't need the destructors in your interfaces. They will generate object code (because virtual methods can't be inlined) and you don't really need to enforce them virtual, as QObject already does that.

                                      I will try to create a minimum test-case that allows you reproducing the problem. However, that might take a couple of days (most likely I'll get it done over the weekend).

                                      If there's nothing secretive about the code (i.e. it's not protected by copyright or something like this) you could upload it in a repo somewhere as it is. I'd download it in the evening and check if I can find the problem.

                                      Kind regards.

                                      J Offline
                                      J Offline
                                      Joel Bodenmann
                                      wrote on 19 Jun 2016, 18:28 last edited by
                                      #21

                                      @kshegunov said:

                                      And by the way you don't need the destructors in your interfaces. They will generate object code (because virtual methods can't be inlined) and you don't really need to enforce them virtual, as QObject already does that.

                                      The reason I added those virtual destructors there is because Qt documenation adviced me to do so: https://doc.qt.io/qt-4.8/qt-tools-plugandpaint-example.html

                                      The class also has a virtual destructor. Interface classes usually don't need such a destructor (because it would make little sense to delete the object that implements the interface through a pointer to the interface), but some compilers emit a warning for classes that declare virtual functions but no virtual destructor. We provide the destructor to keep these compilers happy.
                                      

                                      Industrial process automation software: https://simulton.com
                                      Embedded Graphics & GUI library: https://ugfx.io

                                      K 1 Reply Last reply 19 Jun 2016, 19:12
                                      0
                                      • J Joel Bodenmann
                                        19 Jun 2016, 18:28

                                        @kshegunov said:

                                        And by the way you don't need the destructors in your interfaces. They will generate object code (because virtual methods can't be inlined) and you don't really need to enforce them virtual, as QObject already does that.

                                        The reason I added those virtual destructors there is because Qt documenation adviced me to do so: https://doc.qt.io/qt-4.8/qt-tools-plugandpaint-example.html

                                        The class also has a virtual destructor. Interface classes usually don't need such a destructor (because it would make little sense to delete the object that implements the interface through a pointer to the interface), but some compilers emit a warning for classes that declare virtual functions but no virtual destructor. We provide the destructor to keep these compilers happy.
                                        
                                        K Offline
                                        K Offline
                                        kshegunov
                                        Moderators
                                        wrote on 19 Jun 2016, 19:12 last edited by
                                        #22

                                        @Joel-Bodenmann

                                        The reason I added those virtual destructors there is because Qt documenation adviced me to do so

                                        Right, it's not an error by any means. However, I advise not using substandard compilers instead. ;)

                                        Read and abide by the Qt Code of Conduct

                                        1 Reply Last reply
                                        0

                                        • Login

                                        • Login or register to search.
                                        • First post
                                          Last post
                                        0
                                        • Categories
                                        • Recent
                                        • Tags
                                        • Popular
                                        • Users
                                        • Groups
                                        • Search
                                        • Get Qt Extensions
                                        • Unsolved