clementine-player / Clementine

:tangerine: Clementine Music Player
https://www.clementine-player.org/
GNU General Public License v3.0
3.68k stars 669 forks source link

Ubuntu Unity Hack tries to run on a non Ubuntu distro without Unity #7234

Open raphj opened 1 year ago

raphj commented 1 year ago

Before posting

Please follow the steps below and check the boxes with [x] once you did the step.

System information

Please provide information about your system and the version of Clementine used.

Expected behaviour / actual behaviour

I noticed this by running Clementine in Valgrind and seeing:

==22600== Syscall param waitid(infop) points to unaddressable byte(s)
==22600==    at 0x78B3A8D: syscall (in /usr/lib64/libc.so.6)
==22600==    by 0x5107536: ??? (in /usr/lib64/libQt5Core.so.5.15.7)
==22600==    by 0x50EADFD: ??? (in /usr/lib64/libQt5Core.so.5.15.7)
==22600==    by 0xD70533: UbuntuUnityHack::UbuntuUnityHack(QObject*) (ubuntuunityhack.cpp:47)
==22600==    by 0xA68306: main (main.cpp:448)
==22600==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

Steps to reproduce the problem (only for bugs)

Suggested solution

ubuntuunityhack.cpp tries to know whether we are on Ubuntu. If so, it proceeds to apply its workaround.

The check is as follows: it checks if there is a /etc/lsb release file. If so, and if DISTRIB_ID=Ubuntu is not found, it returns. A probably better way to do it would be to return if /etc/lsb-release file is not found, because Ubuntu should have it.

This can be done by changing:

  // Check if we're on Ubuntu first.
  QFile lsb_release("/etc/lsb-release");
  if (lsb_release.open(QIODevice::ReadOnly)) {
    QByteArray data = lsb_release.readAll();
    if (!data.contains("DISTRIB_ID=Ubuntu")) {
      // It's not Ubuntu - don't do anything.
      return;
    }
  }

to:

  // Check if we're on Ubuntu first.
  QFile lsb_release("/etc/lsb-release");
  if (!lsb_release.open(QIODevice::ReadOnly) || !lsb_release.readAll().contains("DISTRIB_ID=Ubuntu")) {
    // It's not Ubuntu - don't do anything.
    return;
  }

Even better, Clementine could check that Unity is running. Unity could theoretically be running from a distro that's not Ubuntu. I don't have Unity, so I won't be able to contribute a patch that does this.

raphj commented 1 year ago

It is to be noted that after preventing the ubuntu unity hack from running, I see:

==22990== Syscall param waitid(infop) points to unaddressable byte(s)
==22990==    at 0x78B3A8D: syscall (in /usr/lib64/libc.so.6)
==22990==    by 0x5107536: ??? (in /usr/lib64/libQt5Core.so.5.15.7)
==22990==    by 0x50EADFD: ??? (in /usr/lib64/libQt5Core.so.5.15.7)
==22990==    by 0xAD50A8: WorkerPool<AbstractMessageHandler<cpb::tagreader::Message> >::StartOneWorker(WorkerPool<AbstractMessageHandler<cpb::tagreader::Message> >::Worker*) (workerpool.h:296)
==22990==    by 0xAD44EB: WorkerPool<AbstractMessageHandler<cpb::tagreader::Message> >::DoStart() (workerpool.h:252)
==22990==    by 0x1047B2A: _WorkerPoolBase::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (moc_workerpool.cpp:92)
==22990==    by 0x519FC3F: QObject::event(QEvent*) (in /usr/lib64/libQt5Core.so.5.15.7)
==22990==    by 0x66663FD: QApplicationPrivate::notify_helper(QObject*, QEvent*) (in /usr/lib64/libQt5Widgets.so.5.15.7)
==22990==    by 0x5174127: QCoreApplication::notifyInternal2(QObject*, QEvent*) (in /usr/lib64/libQt5Core.so.5.15.7)
==22990==    by 0x51770C0: QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (in /usr/lib64/libQt5Core.so.5.15.7)
==22990==    by 0x51CC352: ??? (in /usr/lib64/libQt5Core.so.5.15.7)
==22990==    by 0x5DC2A8F: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.7400.1)
==22990==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

So the cause of this message is probably elsewhere