Commit 91284ff2 authored by Christian Esken's avatar Christian Esken
Browse files

Add a Lock to avoid duplicate initializiation, which may help with some

startup issues (delays, lockups). Not likely a solutin for all, but
please test. Also some ceanups, less logging and fixing a small memleak.

CCBUGS: 318986
CCBUGS: 339272
CCBUGS: 339525
CCBUGS: 317926
parent 8c7d33c5
......@@ -26,6 +26,7 @@ if(POLICY CMP0046)
cmake_policy (SET CMP0046 NEW)
endif()
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
if (KMIX_KF5_BUILD)
find_package(ECM 0.0.11 REQUIRED NO_MODULE)
......
......@@ -28,9 +28,10 @@
#include "core/ControlManager.h"
#include "core/GlobalConfig.h"
bool KMixApp::firstCaller = true;
KMixApp::KMixApp() :
KUniqueApplication(), m_kmix(0)
KUniqueApplication(), m_kmix(0), creationLock(QMutex::Recursive)
{
GlobalConfig::init();
......@@ -45,108 +46,170 @@ KMixApp::KMixApp() :
KMixApp::~KMixApp()
{
kDebug() << "Deleting KMixApp";
ControlManager::instance().shutdownNow();
delete m_kmix;
m_kmix = 0;
GlobalConfig::shutdown();
}
bool KMixApp::restoreSessionIfApplicable()
void KMixApp::createWindowOnce(bool hasArgKeepvisibility, bool reset)
{
bool restore = isSessionRestored() && KMainWindow::canBeRestored(0);
// Create window, if it was not yet created (e.g. via autostart or manually)
if (m_kmix == 0)
{
kDebug() << "Creating new KMix window";
m_kmix = new KMixWindow(hasArgKeepvisibility, reset);
}
}
bool KMixApp::restoreSessionIfApplicable(bool hasArgKeepvisibility, bool reset)
{
/**
* We should lock session creation. Rationale:
* KMix can be started multiple times during session start. By "autostart" and "session restore". The order is
* undetermined, as KMix will initialize in the background of KDE session startup (Hint: As a
* KUniqueApplication it decouples from the startkde process!).
*
* Now we must make sure that window creation is definitely done, before the "other" process comes, as it might
* want to restore the session. Working on a half-created window would not be smart! Why can this happen? It
* depends on implementation details insinde Qt, which COULD potentially lead to the following scenarios:
* 1) "Autostart" and "session restore" run concurrenty in 2 differnent Threads.
* 2) The current "main/gui" thread "pops up" a "session restore" message from the Qt event dispatcher.
* This means that "Autostart" and "session restore" run interleaved in a single Thread.
*/
creationLock.lock();
bool restore = isSessionRestored(); // && KMainWindow::canBeRestored(0);
kDebug() << "Starting KMix using kepvisibility=" << hasArgKeepvisibility << ", failsafe=" << reset << ", sessionRestore=" << restore;
int createCount = 0;
if (restore)
{
m_kmix->restore(0, false);
if (reset)
{
kWarning() << "Reset cannot be performed while KMix is running. Please quit KMix and retry then.";
}
int n = 1;
while (KMainWindow::canBeRestored(n))
{
kDebug() << "Restoring window " << n;
if (n > 1)
{
// This code path is "impossible". It is here only for analyzing possible issues with session resoring.
// KMix is a single-instance app. If more than one instance is craeated we have a bug.
kWarning() << "KDE session management wants to restore multiple instances of KMix. Please report this as a bug.";
break;
}
else
{
// Create window, if it was not yet created (e.g. via autostart or manually)
createWindowOnce(hasArgKeepvisibility, reset);
// #restore() is called with the parameter of "show == false", as KMixWindow iteself decides on it.
m_kmix->restore(n, false);
createCount++;
}
}
}
if (createCount == 0)
{
// Normal start, or if nothing could be restored
createWindowOnce(hasArgKeepvisibility, reset);
}
creationLock.unlock();
return restore;
}
int KMixApp::newInstance()
{
// There are 3 cases for a new instance
KCmdLineArgs *args = KCmdLineArgs::parsedArgs();
bool hasArgKeepvisibility = args->isSet("keepvisibility");
bool reset = args->isSet("failsafe");
/**
* There are 3 cases when starting KMix:
* Autostart : Cases 1) or 3) below
* Session restore : Cases 1) or 2a) below
* Manual start by user : Cases 1) or 2b) below
*
* Each may be the creator a new instance, but only if the instance did not exist yet.
*/
//kDebug(67100) << "KMixApp::newInstance() isRestored()=" << isRestored() << "_keepVisibility=" << _keepVisibility;
static bool first = true;
if (!first)
/**
* NB See https://qa.mandriva.com/show_bug.cgi?id=56893#c3
*
* It is important to track this via a separate variable and not
* based on m_kmix to handle this race condition.
* Specific protection for the activation-prior-to-full-construction
* case exists above in the 'already running case'
*/
creationLock.lock(); // Guard a complete construction
bool first = firstCaller;
firstCaller = false;
if (first)
{
// There already exists an instance/window
kDebug(67100)
<< "KMixApp::newInstance() Instance exists";
KCmdLineArgs *args = KCmdLineArgs::parsedArgs();
bool hasArgKeepvisibility = args->isSet("keepvisibility");
bool reset = args->isSet("failsafe");
if (reset)
{
kWarning() << "Reset cannot be performed while KMix is running. Please quit KMix and retry then.";
}
/** CASE 1 *******************************************************
*
* Typical case: Normal start. KMix was not running yet => create a new KMixWindow
*/
GlobalConfig::init();
restoreSessionIfApplicable(hasArgKeepvisibility, reset);
}
else
{
if (!hasArgKeepvisibility)
{
// *** CASE 1 ******************************************************
/*
* KMix is running, AND the *USER* starts it again (w/o --keepvisibilty), the KMix main window will be shown.
/** CASE 2 ******************************************************
*
* KMix is running, AND the *USER* starts it again (w/o --keepvisibilty)
* 2a) Restored the KMix main window will be shown.
* 2b) Not restored
*/
kDebug(67100)
<< "KMixApp::newInstance() SHOW WINDOW (_keepVisibility="
<< hasArgKeepvisibility << ", isSessionRestored="
<< isSessionRestored();
/*
* Restore Session. This may look strange to you, as the instance already exists. But the following
* sequence might happen:
* 1) Autostart (no restore) => create m_kmix instance (via CASE 3)
* 2) Session restore => we are here at this line of code (CASE 1). m_kmix exists, but still must be restored
* 1) Autostart (no restore) => create m_kmix instance (via CASE 1)
* 2) Session restore => we are here at this line of code (CASE 2). m_kmix exists, but still must be restored
*
*/
bool wasRestored = restoreSessionIfApplicable();
bool wasRestored = restoreSessionIfApplicable(hasArgKeepvisibility, reset);
// Use standard newInstances(), which shows and activates the main window. But skip it for the
// special "restored" case, as we should not override the session rules.
if (!wasRestored)
{
//
// Use standard newInstances(), which shows and activates the main window. But skip it for the
// special "restored" case, as we should not override the session rules.
KUniqueApplication::newInstance();
}
// else: Do nothing, as session restore has done it.
}
else
{
// *** CASE 2 ******************************************************
/*
* If KMix is running, AND launched again with --keepvisibilty
/** CASE 3 ******************************************************
*
* => We don't want to change the visibiliy, thus we don't call show() here.
* KMix is running, AND launched again with --keepvisibilty
*
* Typical use case: Autostart
*
* Hint: --keepVisibility is a special (legacy) option for applications that want to start
* a mixer service, but don't need to show the KMix GUI (like KMilo , KAlarm, ...).
* See Bug 58901.
* => We don't want to change the visibiliy, thus we don't call show() here.
*
* Nowadays this switch can be considered legacy, as applications should use KMixD instead.
* Hint: --keepVisibility is used in kmix_autostart.desktop. It was used in history by KMilo
* (see BKO 58901), but nowadays Mixer Applets nmight want to use it, though they should
* use KMixD instead.
*/
kDebug(67100)
kDebug()
<< "KMixApp::newInstance() REGULAR_START _keepVisibility="
<< hasArgKeepvisibility;
}
}
else
{
// *** CASE 3 ******************************************************
/*
* Regular case: KMix was not running yet => create a new KMixWindow
*/
first = false;// NB See https://qa.mandriva.com/show_bug.cgi?id=56893#c3
// It is important to track this via a separate variable and not
// based on m_kmix to handle this race condition.
// Specific protection for the activation-prior-to-full-construction
// case exists above in the 'already running case'
GlobalConfig::init();
KCmdLineArgs *args = KCmdLineArgs::parsedArgs();
bool hasArgKeepvisibility = args->isSet("keepvisibility");
bool reset = args->isSet("failsafe");
m_kmix = new KMixWindow(hasArgKeepvisibility, reset);
restoreSessionIfApplicable();
}
creationLock.unlock();
return 0;
}
......
......@@ -21,6 +21,7 @@
#ifndef KMixApp_h
#define KMixApp_h
#include <QMutex>
#include <kuniqueapplication.h>
class KMixWindow;
......@@ -28,23 +29,19 @@ class KMixWindow;
class KMixApp : public KUniqueApplication
{
Q_OBJECT
bool restoreSessionIfApplicable();
bool restoreSessionIfApplicable(bool hasArgKeepvisibility, bool reset);
void createWindowOnce(bool hasArgKeepvisibility, bool reset);
public:
KMixApp();
~KMixApp();
int newInstance ();
int newInstance() override;
public slots:
//void quitExtended(); // For a hack on visibility()
// static void keepVisibility(bool);
/*
signals:
void stopUpdatesOnVisibility();
*/
private:
KMixWindow *m_kmix;
// static bool _keepVisibility;
QMutex creationLock;
static bool firstCaller;
};
#endif
......@@ -566,6 +566,7 @@ void KMixWindow::loadBaseConfig()
// --- Advanced options, without GUI: END -------------------------------------
// The following log is very helpful in bug reports. Please keep it.
m_backendFilter = config.readEntry<>("Backends", QList<QString>());
kDebug()
<< "Backends: " << m_backendFilter;
......@@ -686,17 +687,11 @@ void KMixWindow::recreateGUI(bool saveConfig, const QString& mixerId, bool force
{
GUIProfile* guiprof = 0;
// if (mixer->isDynamic())
// {
// // Dynamic will ALWAYS get the fallbackProfile. Actually user can not disable it
// guiprof = GUIProfile::fallbackProfile(mixer);
// }
// else
if (reset)
{
guiprof = GUIProfile::find(mixer, QString("default"), false, true); // ### Card unspecific profile ###
}
// else
if ( guiprof != 0 )
{
guiprof->setDirty(); // All fallback => dirty
......
......@@ -90,5 +90,6 @@ kdemain(int argc, char *argv[])
KMixApp *app = new KMixApp();
int ret = app->exec();
delete app;
kDebug() << "KMix is now exiting, status=" << ret;
return ret;
}
......@@ -620,7 +620,7 @@ void MPrisControl::trackChangedIncoming(QVariantMap /*msg*/)
MediaController::PlayState Mixer_MPRIS2::mprisPlayStateString2PlayState(const QString& playbackStatus)
{
MediaController::PlayState playState;
MediaController::PlayState playState = MediaController::PlayStopped; // presume Stopped for unknown state
if (playbackStatus == "Playing")
{
playState = MediaController::PlayPlaying;
......
......@@ -260,7 +260,7 @@ static void source_cb(pa_context *c, const pa_source_info *i, int eol, void *) {
// Do something....
if (PA_INVALID_INDEX != i->monitor_of_sink)
{
kDebug(67100) << "Ignoring Monitor Source: " << i->description;
// kDebug(67100) << "Ignoring Monitor Source: " << i->description;
return;
}
......@@ -929,9 +929,9 @@ bool Mixer_PULSE::addDevice(devinfo& dev, bool isAppStream)
if (isAppStream)
md->setApplicationStream(true);
kDebug(67100) << "Adding Pulse volume " << dev.name << ", isCapture= "
<< (m_devnum == KMIXPA_CAPTURE || m_devnum == KMIXPA_APP_CAPTURE)
<< ", isAppStream= " << isAppStream << "=" << md->isApplicationStream() << ", devnum=" << m_devnum;
// kDebug(67100) << "Adding Pulse volume " << dev.name << ", isCapture= "
// << (m_devnum == KMIXPA_CAPTURE || m_devnum == KMIXPA_APP_CAPTURE)
// << ", isAppStream= " << isAppStream << "=" << md->isApplicationStream() << ", devnum=" << m_devnum;
md->addPlaybackVolume(v);
md->setMuted(dev.mute);
m_mixDevices.append(md->addToPool());
......
......@@ -31,8 +31,9 @@ GlobalConfig::GlobalConfig() :
addItemBool("Labels", data.showLabels, true);
addItemBool("VolumeOverdrive", data.volumeOverdrive, false);
addItemBool("VolumeFeedback", data.beepOnVolumeChange, true);
ItemString* is = addItemString("Orientation", data.orientationMainGUIString, "Vertical");
kDebug() << is->name() << is->value();
// ItemString* is =
addItemString("Orientation", data.orientationMainGUIString, "Vertical");
// kDebug() << is->name() << is->value();
addItemString("Orientation.TrayPopup", data.orientationTrayPopupString, QLatin1String("Vertical"));
// Sound Menu
......@@ -50,6 +51,7 @@ GlobalConfig::GlobalConfig() :
addItemBool("Debug.ControlManager", data.debugControlManager, false);
addItemBool("Debug.GUI", data.debugGUI, false);
addItemBool("Debug.Volume", data.debugVolume, false);
addItemBool("Debug.Config", data.debugConfig, false);
readConfig();
}
......
......@@ -51,6 +51,7 @@ public:
bool debugControlManager;
bool debugGUI;
bool debugVolume;
bool debugConfig;
Qt::Orientation getToplevelOrientation();
Qt::Orientation getTraypopupOrientation();
......@@ -95,6 +96,18 @@ public:
}
;
/**
* Frees all data associated with the static instance.
*
*/
static void shutdown()
{
delete instanceObj;
instanceObj = 0;
}
;
GlobalConfigData data;
void setMixersForSoundmenu(QSet<QString> mixersForSoundmenu)
{
......
......@@ -288,6 +288,7 @@ MixDevice::~MixDevice()
{
_enumValues.clear(); // The QString's inside will be auto-deleted, as they get unref'ed
delete _dbusControlWrapper;
delete mediaController;
}
Volume& MixDevice::playbackVolume()
......@@ -425,7 +426,7 @@ void MixDevice::readPlaybackOrCapture(const KConfigGroup& config, bool capture)
bool MixDevice::write( KConfig *config, const QString& grp )
{
if (_mixer->isDynamic() || isArtificial()) {
kDebug(67100) << "MixDevice::write(): This MixDevice does not permit volume saving (i.e. because it is handled lower down in the audio stack). Ignoring.";
// kDebug(67100) << "MixDevice::write(): This MixDevice does not permit volume saving (i.e. because it is handled lower down in the audio stack). Ignoring.";
return false;
}
......
......@@ -509,7 +509,7 @@ MasterControl& Mixer::getGlobalMasterPreferred(bool fallbackAllowed)
static MasterControl result;
if ( !fallbackAllowed || _globalMasterPreferred.isValid() ) {
kDebug() << "Returning preferred master";
// kDebug() << "Returning preferred master";
return _globalMasterPreferred;
}
......@@ -517,7 +517,7 @@ MasterControl& Mixer::getGlobalMasterPreferred(bool fallbackAllowed)
if (mm) {
result.set(_globalMasterPreferred.getCard(), mm->getRecommendedDeviceId());
if (!result.getControl().isEmpty())
kDebug() << "Returning extended preferred master";
// kDebug() << "Returning extended preferred master";
return result;
}
......
......@@ -302,7 +302,8 @@ void KMixPrefDlg::addWidgetToLayout(QWidget* widget, QBoxLayout* layout, int spa
*/
void KMixPrefDlg::updateWidgets()
{
kDebug() << "";
if (dialogConfig.data.debugConfig)
kDebug() << "";
bool toplevelHorizontal = dialogConfig.data.getToplevelOrientation() == Qt::Horizontal;
_rbHorizontal->setChecked(toplevelHorizontal);
_rbVertical->setChecked(!toplevelHorizontal);
......@@ -320,11 +321,13 @@ void KMixPrefDlg::updateWidgets()
void KMixPrefDlg::updateSettings()
{
Qt::Orientation toplevelOrientation = _rbHorizontal->isChecked() ? Qt::Horizontal : Qt::Vertical;
kDebug() << "toplevelOrientation" << toplevelOrientation << ", _rbHorizontal->isChecked()" << _rbHorizontal->isChecked();
if (dialogConfig.data.debugConfig)
kDebug() << "toplevelOrientation" << toplevelOrientation << ", _rbHorizontal->isChecked()" << _rbHorizontal->isChecked();
dialogConfig.data.setToplevelOrientation(toplevelOrientation);
Qt::Orientation trayOrientation = _rbTraypopupHorizontal->isChecked() ? Qt::Horizontal : Qt::Vertical;
kDebug() << "trayOrientation" << trayOrientation << ", _rbTraypopupHorizontal->isChecked()" << _rbTraypopupHorizontal->isChecked();
if (dialogConfig.data.debugConfig)
kDebug() << "trayOrientation" << trayOrientation << ", _rbTraypopupHorizontal->isChecked()" << _rbTraypopupHorizontal->isChecked();
dialogConfig.data.setTraypopupOrientation(trayOrientation);
// Announcing MasterChanged, as the sound menu (aka ViewDockAreaPopup) primarily shows master volume(s).
......@@ -355,14 +358,16 @@ bool KMixPrefDlg::hasChanged()
{
bool orientationFromConfigIsHor = dialogConfig.data.getToplevelOrientation() == Qt::Horizontal;
bool orientationFromWidgetIsHor = _rbHorizontal->isChecked();
kDebug() << "Orientation MAIN fromConfig=" << (orientationFromConfigIsHor ? "Hor" : "Vert") << ", fromWidget=" << (orientationFromWidgetIsHor ? "Hor" : "Vert");
if (dialogConfig.data.debugConfig)
kDebug() << "Orientation MAIN fromConfig=" << (orientationFromConfigIsHor ? "Hor" : "Vert") << ", fromWidget=" << (orientationFromWidgetIsHor ? "Hor" : "Vert");
bool changed = orientationFromConfigIsHor ^ orientationFromWidgetIsHor;
if (!changed)
{
bool orientationFromConfigIsHor = dialogConfig.data.getTraypopupOrientation() == Qt::Horizontal;
orientationFromWidgetIsHor = _rbTraypopupHorizontal->isChecked();
kDebug() << "Orientation TRAY fromConfig=" << (orientationFromConfigIsHor ? "Hor" : "Vert") << ", fromWidget=" << (orientationFromWidgetIsHor ? "Hor" : "Vert");
if (dialogConfig.data.debugConfig)
kDebug() << "Orientation TRAY fromConfig=" << (orientationFromConfigIsHor ? "Hor" : "Vert") << ", fromWidget=" << (orientationFromWidgetIsHor ? "Hor" : "Vert");
changed = orientationFromConfigIsHor ^ orientationFromWidgetIsHor;
}
......@@ -371,7 +376,8 @@ bool KMixPrefDlg::hasChanged()
changed = dvc->getModifyFlag();
}
kDebug() << "hasChanged=" << changed;
if (dialogConfig.data.debugConfig)
kDebug() << "hasChanged=" << changed;
return changed;
}
......@@ -405,7 +411,8 @@ void KMixPrefDlg::showEvent(QShowEvent * event)
"autostart",
#endif
QString("kmix_autostart.desktop"));
kDebug() << "autostartConfigFilename = " << autostartConfigFilename;
if (dialogConfig.data.debugConfig)
kDebug() << "autostartConfigFilename = " << autostartConfigFilename;
bool autostartFileExists = !autostartConfigFilename.isNull();
//allowAutostartWarning->setEnabled(autostartFileExists);
......
......@@ -406,12 +406,12 @@ ProfControl* ViewBase::findMdw(const QString& mdwId, GuiVisibility visibility)
{
if (pControl->getVisibility().satisfiesVisibility(visibility))
{
kDebug() << " MATCH " << (*pControl).id << " for " << mdwId << " with visibility " << pControl->getVisibility().getId() << " to " << visibility.getId();
// kDebug() << " MATCH " << (*pControl).id << " for " << mdwId << " with visibility " << pControl->getVisibility().getId() << " to " << visibility.getId();
return pControl;
}
else
{
kDebug() << "NOMATCH " << (*pControl).id << " for " << mdwId << " with visibility " << pControl->getVisibility().getId() << " to " << visibility.getId();
// kDebug() << "NOMATCH " << (*pControl).id << " for " << mdwId << " with visibility " << pControl->getVisibility().getId() << " to " << visibility.getId();
}
}
} // iterate over all ProfControl entries
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment