[phonon/4.7] phonon: allow pulsesupport to return null on instance requests

[Harald Sitter]

Git commit 113dc5101cb96f1d3c94650fb34aa5fcca973595 by Harald Sitter.
Committed on 01/08/2014 at 15:19.
Pushed by sitter into branch '4.7'.

allow pulsesupport to return null on instance requests

it can happen that we destruct our factory (and along with it all backend
objects and pulsesupport) before all frontend classes have been
deleted. most notably this currently happens with knotifications that
old the frontend objects in a q_global_static that is only cleaned up
in the applications' exit handler. at that point our frontend class
would then try to access an already destructed pulsesupport and causing
a dummy instance to be created which ultimately didn't do anything anyway.

to prevent this from happen pulsesupport now has a new getInstanceOrNull
which can be explicitly instructed to return null iff the instance
was already shut down, since this now requires explicit nullptr handling
this new functionality is separate from the regular getInstance which
always returns a valid pointer (albeit sometimes to a dummy instance).

this new getInstanceOrNull function is used in our frontend audiooutput
to prevent dummy creation on destruction. since the AO only clears the
phonon internal stream cache (mapping AOs to pulse stream ids) we do not
have to do this if the instance has already been shut down anyway
ultimately making this part of the code non-essential.

(random related note: phonon's factory cleans up on qapplication
 destruction, so we can safely assume that consumers will not try to
 actively use our classes past this point, and if they do it's really their
 fault if something goes wrong)

CCMAIL: martin.klapetek at gmail.com
CCMAIL: kde-multimedia at kde.org

M  +4    -1    phonon/audiooutput.cpp
M  +12   -2    phonon/pulsesupport.cpp
M  +14   -1    phonon/pulsesupport.h

http://commits.kde.org/phonon/113dc5101cb96f1d3c94650fb34aa5fcca973595

diff --git a/phonon/audiooutput.cpp b/phonon/audiooutput.cpp
index dd33364..0768768 100644
--- a/phonon/audiooutput.cpp
+++ b/phonon/audiooutput.cpp
@@ -532,7 +532,10 @@ void AudioOutputPrivate::handleAutomaticDeviceChange(const AudioOutputDevice &de
 
 AudioOutputPrivate::~AudioOutputPrivate()
 {
-    PulseSupport::getInstance()->clearStreamCache(streamUuid);
+    PulseSupport *pulse = PulseSupport::getInstanceOrNull(true);
+    if (pulse) {
+        pulse->clearStreamCache(streamUuid);
+    }
 #ifndef PHONON_NO_DBUS
     if (adaptor) {
         emit adaptor->outputDestroyed();
diff --git a/phonon/pulsesupport.cpp b/phonon/pulsesupport.cpp
index f55d0b4..a190d1d 100644
--- a/phonon/pulsesupport.cpp
+++ b/phonon/pulsesupport.cpp
@@ -51,6 +51,7 @@ namespace Phonon
 
 QMutex probeMutex;
 static PulseSupport *s_instance = NULL;
+static bool s_wasShutDown = false;
 
 #ifdef HAVE_PULSEAUDIO
 /***
@@ -771,9 +772,12 @@ static void context_state_callback(pa_context *c, void *)
 }
 #endif // HAVE_PULSEAUDIO
 
-
-PulseSupport *PulseSupport::getInstance()
+PulseSupport *PulseSupport::getInstanceOrNull(bool allowNull)
 {
+    if (s_wasShutDown && allowNull) {
+        return NULL;
+    }
+
     if (NULL == s_instance) {
         /*
          * In order to prevent the instance being used from multiple threads
@@ -789,11 +793,17 @@ PulseSupport *PulseSupport::getInstance()
     return s_instance;
 }
 
+PulseSupport *PulseSupport::getInstance()
+{
+    return getInstanceOrNull(false);
+}
+
 void PulseSupport::shutdown()
 {
     if (NULL != s_instance) {
         delete s_instance;
         s_instance = NULL;
+        s_wasShutDown = true;
     }
 }
 
diff --git a/phonon/pulsesupport.h b/phonon/pulsesupport.h
index 6df47e2..19a83c0 100644
--- a/phonon/pulsesupport.h
+++ b/phonon/pulsesupport.h
@@ -38,7 +38,20 @@ namespace Phonon
     {
         Q_OBJECT
         public:
-            static PulseSupport* getInstance();
+            /**
+             * \returns the instance pointer or null, see note.
+             * \note If \param allowNull is \c true and the instance was already
+             *       shut down this function instead returns to indicate that
+             *       the instance was already shut down.
+             *       If \param allowNull is \c false and the instance was already
+             *       shut down a dummy instance is returned instead. This case
+             *       will furthermore result in a qWarning being printed, so
+             *       when possible and sensible null handling should be done
+             *       to prevent this.
+             */
+            static PulseSupport *getInstanceOrNull(bool allowNull = false);
+            /** This function behaves like getInstanceOrNull(false). \see getInstanceOrNull */
+            static PulseSupport *getInstance();
             static void shutdown();
 
             bool isActive();