When program quit, QSystemTrayIcon crashed.

ugiwgh · wrote on 18 Mar 2016, 10:54

The code is following

class Tray : public QSystemTrayIcon
{
  Q_OBJECT
public:
  Tray();
  ~Tray();
  static Tray* instance()
	{
		if(NULL==instance_)
		{
			instance_=new PTray();
		}
		return instance_;
  }
private:
  class Garbo
  {
  public:
    ~Garbo()
    {
      if(instance_)
      {
        delete instance_;
      }
    }
  };
  static Garbo garbo_;
  static Tray *instance_;
};

(edited to add code tags: mrjj)

The gdb bt is following:
#0 0x00000000004529fa in QWidget::~QWidget() ()
#1 0x00000000008b2389 in QBalloonTip::~QBalloonTip() ()
#2 0x00000000008b16aa in QBalloonTip::hideBalloon() ()
#3 0x00000000008c1813 in QSystemTrayIconPrivate::remove_sys() ()
#4 0x00000000008b25cf in QSystemTrayIcon::~QSystemTrayIcon() ()
#5 0x0000000000411d50 in PTray::~PTray (this=0x1d02c00, __in_chrg=<value optimized out>)
at para_tray.cpp:21
#6 0x0000000000411d86 in PTray::~PTray (this=0x1d02c00, __in_chrg=<value optimized out>)
at para_tray.cpp:21
#7 0x00000000004122d3 in PTray::Garbo::~Garbo (this=0x118edb0,
__in_chrg=<value optimized out>) at ./para_tray.h:26
#8 0x0000003c82235fd2 in exit () from /lib64/libc.so.6
#9 0x0000003c8221ec64 in __libc_start_main () from /lib64/libc.so.6
#10 0x000000000040e9d9 in _start ()

SGaist · 21 Mar 2016, 01:43

Hi and welcome to devnet,

Why all these singletons ?

ugiwgh · 21 Mar 2016, 02:31

@SGaist I want it new once, and call it in some other functions. It seems that it carsh, does not because of singletons. I new QTray, and show it, and then delete it, and then it crash.

ugiwgh · wrote on 21 Mar 2016, 02:12

When I hide tray, and then delete, it does not crash.

kshegunov · 21 Mar 2016, 09:52

@ugiwgh
Your problem is you're calling destructors of QObject instances, after the root QObject (i.e. QApplication) has been destroyed. You just can't, shouldn't and mustn't do that. So yes, the singleton is responsible for that bloody mess, because variables with global storage are allocated before (but not necessarily constructed) and destroyed after main().

One more remark:
That trick with having a special object for cleanup is very tricky. You can lose the pointer to the object before invoking the delete operator, so you can be deleting a memory location pointed by an invalid variable that may (or may not) still hold a valid address; it would depend on the compiler implementation.
I haven't actually checked the standard to see whether integral types are freed after objects with non-trivial destructors, but whatsoever it says, this way - to depend on the order of construction/destruction of static variables - is a terrible technique.

ugiwgh · wrote on 21 Mar 2016, 09:52

@kshegunov I think you are right. Thanks for your help.

ugiwgh · 21 Mar 2016, 12:05

@kshegunov

It is nessary to add "instance_=NULL;" into destructor?

class Tray : public QSystemTrayIcon
{
Q_OBJECT
public:
~Tray() { instance_=NULL; }
static Tray* instance()
{
if(NULL==instance_)
{
instance_=new PTray();
}
return instance_;
}
private:
Tray(){}
static Tray *instance_;
};

kshegunov · 22 Mar 2016, 03:14

@ugiwgh

It is nessary to add "instance_=NULL;" into destructor?

Well, no. This does nothing, as delete NULL is valid and does nothing. The object will be still in memory anyway. Beside refactoring your code and not using globally available objects (which the singleton is - basically a global variable) you can hook up the destruction to be tied to Qt's application object's life cycle. To do that you just parent the QObject you're creating to the QApplication object like this:

class Tray : public QSystemTrayIcon
{
    Q_OBJECT

public:
    Tray(QObject *);
    ~Tray();

    static Tray * instance();
};

Tray::Tray(QObject * parent = NULL)
    : QSystemTrayIcon(parent)
{
}

Tray::~Tray()
{
    // Don't delete the object. Qt will delete it when the parent goes out of scope.
}

Tray * Tray::instance()
{
    QCoreApplication * app = QCoreApplication::instance();
    Q_ASSERT(app); //< Make sure this is not called before the application was constructed.
    
    static Tray * trayInstance = new Tray(app);
    return trayInstance;
}

Again, it's better to not have singletons at all, C++ is not Java and not everything that's written in books about Java (or is common there) is applicable to C++. But the above snippet should work, although I haven't tested it.

ugiwgh · 23 Mar 2016, 05:08

@kshegunov
I have tested that. It crashed.
If I add tray->hide() after app.exec(). It normal exit.

(gdb) bt
#0 0x000000000045821a in QWidget::~QWidget() ()
#1 0x00000000008b79a9 in QBalloonTip::~QBalloonTip() ()
#2 0x00000000008b6cca in QBalloonTip::hideBalloon() ()
#3 0x00000000008c6e33 in QSystemTrayIconPrivate::remove_sys() ()
#4 0x00000000008b7bef in QSystemTrayIcon::~QSystemTrayIcon() ()
#5 0x00000000004124b9 in Tray::~Tray (this=0x2878cc0, __in_chrg=<value optimized out>)
at tray.cpp:26
#6 0x00000000004124f0 in Tray::~Tray (this=0x2878cc0, __in_chrg=<value optimized out>)
at tray.cpp:26
#7 0x0000000000ba1f67 in QObjectPrivate::deleteChildren() ()
#8 0x0000000000ba84b6 in QObject::~QObject() ()
#9 0x000000000042568c in QApplication::~QApplication() ()
#10 0x00000000004121af in QtSingleApplication::~QtSingleApplication (this=0x7fff44a4b6f0,
__in_chrg=<value optimized out>) at ../qtsingleapplication/src/qtsingleapplication.h:65
#11 0x0000000000411c8f in main (argc=1, argv=0x7fff44a4b898) at main.cpp:103

kshegunov · 23 Mar 2016, 11:08

@ugiwgh
Can you provide a minimal project that reproduces the crash (e.g. in a git repository somewhere)?

That's strange, but what's stranger is this:

#10 0x00000000004121af in QtSingleApplication::~QtSingleApplication (this=0x7fff44a4b6f0,
__in_chrg=<value optimized out>) at ../qtsingleapplication/src/qtsingleapplication.h:65

What is this and how it got there?

ugiwgh · 24 Mar 2016, 01:46

@kshegunov
I have upload it there.
https://github.com/ugiwgh/qsystemtray/
https://github.com/ugiwgh/qsystemtray.git

kshegunov · 24 Mar 2016, 05:28

@ugiwgh
Well works okay on my machine - no crashes. I even see an empty space in the tray where when I click I get a context menu and I can choose to quit. When I do, everything works as expected and the program exits.

However, you shouldn't use QtSingleApplication as it seems to be ancient, you'd be better off looking for newer alternatives.

ugiwgh · 24 Mar 2016, 07:06

@kshegunov
Yes. I have disabled QtSingleApplication. And I have update the code on github.
It also crashed, when I choose to quit.

#0 0x00007f094a93564a in QWidget::~QWidget() ()
from /opt/qt-5.4.1/5.4/gcc_64/lib/libQt5Widgets.so.5
#1 0x00007f094ac92f0e in ?? () from /opt/qt-5.4.1/5.4/gcc_64/lib/libQt5Widgets.so.5
#2 0x00007f094ac92b25 in ?? () from /opt/qt-5.4.1/5.4/gcc_64/lib/libQt5Widgets.so.5
#3 0x00007f094ac735eb in QSystemTrayIcon::~QSystemTrayIcon() ()
from /opt/qt-5.4.1/5.4/gcc_64/lib/libQt5Widgets.so.5
#4 0x0000000000403625 in PTray::~PTray (this=0x22a1e60,
__in_chrg=<value optimized out>) at para_tray.cpp:26
#5 0x000000000040365c in PTray::~PTray (this=0x22a1e60,
__in_chrg=<value optimized out>) at para_tray.cpp:26
#6 0x00007f0949ba4b1a in QObjectPrivate::deleteChildren() ()
from /opt/qt-5.4.1/5.4/gcc_64/lib/libQt5Core.so.5
#7 0x00007f0949baa259 in QObject::~QObject() ()
from /opt/qt-5.4.1/5.4/gcc_64/lib/libQt5Core.so.5
#8 0x00007f094a8edda8 in QApplication::~QApplication() ()
from /opt/qt-5.4.1/5.4/gcc_64/lib/libQt5Widgets.so.5
#9 0x0000000000403129 in main (argc=1, argv=0x7ffff084acf8) at main.cpp:23

kshegunov · U ugiwgh 24 Mar 2016, 05:28

@ugiwgh
Well, I don't know man. As I said, I had run your code (the link you provided in your previous post) and it worked okay on my machine, no crashes whatsoever.
If it helps I'm running Debian stretch (4.4.0 kernel) + KDE plasma desktop and I used Qt 5.5.1 for the test run. You may have stumbled on a bug or possibly some platform dependent code is not executing properly, I can't tell really.

EDIT:
I only just now saw something:

#4 0x0000000000403625 in PTray::~PTray (this=0x22a1e60,
__in_chrg=<value optimized out>) at para_tray.cpp:26
#5 0x000000000040365c in PTray::~PTray (this=0x22a1e60,
__in_chrg=<value optimized out>) at para_tray.cpp:26

Why do you have 2 calls to your destructor??!

SGaist · wrote on 24 Mar 2016, 20:34

Like @kshegunov already wrote using singleton should be avoided when possible and your use case really does't need any singleton.

It sounds that you are rather creating tight coupling between your different classes rather that have a lean path to use system tray.

So what are all these classes doing with the system tray icon and that menu ?