upnp: initialize the client handle only once

Eliminate class LibUPnP and move the code to ClientInit.cxx.  Its
initialization function can be called multiple times, but
UpnpRegisterClient() is called at most once.

4 files changed, 73 insertions, 70 deletions

diff --git a/Makefile.am b/Makefile.am
index ebba8846f..0acca6b31 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -218,13 +218,13 @@ src_mpd_SOURCES = \
 
 UPNP_SOURCES = \
 	src/lib/upnp/Init.cxx src/lib/upnp/Init.hxx \
+	src/lib/upnp/ClientInit.cxx src/lib/upnp/ClientInit.hxx \
 	src/lib/upnp/Device.cxx src/lib/upnp/Device.hxx \
 	src/lib/upnp/ContentDirectoryService.cxx src/lib/upnp/ContentDirectoryService.hxx \
 	src/lib/upnp/Discovery.cxx src/lib/upnp/Discovery.hxx \
 	src/lib/upnp/Domain.cxx src/lib/upnp/Domain.hxx \
 	src/lib/upnp/ixmlwrap.cxx src/lib/upnp/ixmlwrap.hxx \
 	src/lib/upnp/Callback.hxx \
-	src/lib/upnp/upnpplib.cxx src/lib/upnp/upnpplib.hxx \
 	src/lib/upnp/Util.cxx src/lib/upnp/Util.hxx \
 	src/lib/upnp/WorkQueue.hxx \
 	src/lib/upnp/Action.hxx
diff --git a/src/db/plugins/UpnpDatabasePlugin.cxx b/src/db/plugins/UpnpDatabasePlugin.cxx
index ac2055f7d..efc602be6 100644
--- a/src/db/plugins/UpnpDatabasePlugin.cxx
+++ b/src/db/plugins/UpnpDatabasePlugin.cxx
@@ -20,7 +20,7 @@
 #include "config.h"
 #include "UpnpDatabasePlugin.hxx"
 #include "lib/upnp/Domain.hxx"
-#include "lib/upnp/upnpplib.hxx"
+#include "lib/upnp/ClientInit.hxx"
 #include "lib/upnp/Discovery.hxx"
 #include "lib/upnp/ContentDirectoryService.hxx"
 #include "lib/upnp/Util.hxx"
@@ -69,7 +69,7 @@ public:
 };
 
 class UpnpDatabase : public Database {
-	LibUPnP *lib;
+	UpnpClient_Handle handle;
 	UPnPDeviceDirectory *discovery;
 
 public:
@@ -175,17 +175,13 @@ UpnpDatabase::Configure(const config_param &, Error &)
 bool
 UpnpDatabase::Open(Error &error)
 {
-	lib = new LibUPnP();
-	if (!lib->ok()) {
-		error.Set(lib->GetInitError());
-		delete lib;
+	if (!UpnpClientGlobalInit(handle, error))
 		return false;
-	}
 
-	discovery = new UPnPDeviceDirectory(lib->getclh());
+	discovery = new UPnPDeviceDirectory(handle);
 	if (!discovery->Start(error)) {
 		delete discovery;
-		delete lib;
+		UpnpClientGlobalFinish();
 		return false;
 	}
 
@@ -199,7 +195,7 @@ void
 UpnpDatabase::Close()
 {
 	delete discovery;
-	delete lib;
+	UpnpClientGlobalFinish();
 }
 
 void
@@ -277,7 +273,7 @@ UpnpDatabase::SearchSongs(const ContentDirectoryService &server,
 		return true;
 
 	std::list<std::string> searchcaps;
-	if (!server.getSearchCapabilities(lib->getclh(), searchcaps, error))
+	if (!server.getSearchCapabilities(handle, searchcaps, error))
 		return false;
 
 	if (searchcaps.empty())
@@ -344,7 +340,7 @@ UpnpDatabase::SearchSongs(const ContentDirectoryService &server,
 		}
 	}
 
-	return server.search(lib->getclh(),
+	return server.search(handle,
 			     objid, cond.c_str(), dirbuf,
 			     error);
 }
@@ -431,7 +427,7 @@ UpnpDatabase::ReadNode(const ContentDirectoryService &server,
 		       Error &error) const
 {
 	UPnPDirContent dirbuf;
-	if (!server.getMetadata(lib->getclh(), objid, dirbuf, error))
+	if (!server.getMetadata(handle, objid, dirbuf, error))
 		return false;
 
 	if (dirbuf.objects.size() == 1) {
@@ -485,8 +481,6 @@ UpnpDatabase::Namei(const ContentDirectoryService &server,
 		return true;
 	}
 
-	const UpnpClient_Handle handle = lib->getclh();
-
 	std::string objid(rootid);
 
 	// Walk the path elements, read each directory and try to find the next one
@@ -663,7 +657,7 @@ UpnpDatabase::VisitServer(const ContentDirectoryService &server,
 	   and loop here, but it's not useful as mpd will only return
 	   data to the client when we're done anyway. */
 	UPnPDirContent dirbuf;
-	if (!server.readDir(lib->getclh(), tdirent.m_id.c_str(), dirbuf,
+	if (!server.readDir(handle, tdirent.m_id.c_str(), dirbuf,
 			    error))
 		return false;
 
diff --git a/src/lib/upnp/upnpplib.cxx b/src/lib/upnp/ClientInit.cxx
index d36fc3593..77d9cf03d 100644
--- a/src/lib/upnp/upnpplib.cxx
+++ b/src/lib/upnp/ClientInit.cxx
@@ -18,31 +18,21 @@
  */
 
 #include "config.h"
-#include "upnpplib.hxx"
+#include "ClientInit.hxx"
+#include "Init.hxx"
 #include "Callback.hxx"
 #include "Domain.hxx"
-#include "Init.hxx"
-#include "Log.hxx"
+#include "thread/Mutex.hxx"
+#include "util/Error.hxx"
 
 #include <upnp/upnptools.h>
 
-LibUPnP::LibUPnP()
-{
-	if (!UpnpGlobalInit(init_error))
-		return;
-
-	auto code = UpnpRegisterClient(o_callback, nullptr, &m_clh);
-	if (code != UPNP_E_SUCCESS) {
-		UpnpGlobalFinish();
-		init_error.Format(upnp_domain, code,
-				  "UpnpRegisterClient() failed: %s",
-				  UpnpGetErrorMessage(code));
-		return;
-	}
-}
+static Mutex upnp_client_init_mutex;
+static unsigned upnp_client_ref;
+static UpnpClient_Handle upnp_client_handle;
 
-int
-LibUPnP::o_callback(Upnp_EventType et, void* evp, void* cookie)
+static int
+UpnpClientCallback(Upnp_EventType et, void *evp, void *cookie)
 {
 	if (cookie == nullptr)
 		/* this is the cookie passed to UpnpRegisterClient();
@@ -54,7 +44,50 @@ LibUPnP::o_callback(Upnp_EventType et, void* evp, void* cookie)
 	return callback.Invoke(et, evp);
 }
 
-LibUPnP::~LibUPnP()
+static bool
+DoInit(Error &error)
+{
+	auto code = UpnpRegisterClient(UpnpClientCallback, nullptr,
+				       &upnp_client_handle);
+	if (code != UPNP_E_SUCCESS) {
+		error.Format(upnp_domain, code,
+			     "UpnpRegisterClient() failed: %s",
+			     UpnpGetErrorMessage(code));
+		return false;
+	}
+
+	return true;
+}
+
+bool
+UpnpClientGlobalInit(UpnpClient_Handle &handle, Error &error)
+{
+	if (!UpnpGlobalInit(error))
+		return false;
+
+	upnp_client_init_mutex.lock();
+	bool success = upnp_client_ref > 0 || DoInit(error);
+	upnp_client_init_mutex.unlock();
+
+	if (success) {
+		++upnp_client_ref;
+		handle = upnp_client_handle;
+	} else
+		UpnpGlobalFinish();
+
+	return success;
+}
+
+void
+UpnpClientGlobalFinish()
 {
+	upnp_client_init_mutex.lock();
+
+	assert(upnp_client_ref > 0);
+	if (--upnp_client_ref == 0)
+		UpnpUnRegisterClient(upnp_client_handle);
+
+	upnp_client_init_mutex.unlock();
+
 	UpnpGlobalFinish();
 }
diff --git a/src/lib/upnp/upnpplib.hxx b/src/lib/upnp/ClientInit.hxx
index cd1762ec4..645e64ca6 100644
--- a/src/lib/upnp/upnpplib.hxx
+++ b/src/lib/upnp/ClientInit.hxx
@@ -17,43 +17,19 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-#ifndef _LIBUPNP_H_X_INCLUDED_
-#define _LIBUPNP_H_X_INCLUDED_
+#ifndef MPD_UPNP_CLIENT_INIT_HXX
+#define MPD_UPNP_CLIENT_INIT_HXX
 
-#include "util/Error.hxx"
+#include "check.h"
 
 #include <upnp/upnp.h>
 
-/** Our link to libupnp. Initialize and keep the handle around */
-class LibUPnP {
-	Error init_error;
-	UpnpClient_Handle m_clh;
+class Error;
 
-	static int o_callback(Upnp_EventType, void *, void *);
+bool
+UpnpClientGlobalInit(UpnpClient_Handle &handle, Error &error);
 
-public:
-	LibUPnP();
+void
+UpnpClientGlobalFinish();
 
-	LibUPnP(const LibUPnP &) = delete;
-	LibUPnP &operator=(const LibUPnP &) = delete;
-
-	~LibUPnP();
-
-	/** Check state after initialization */
-	bool ok() const
-	{
-		return !init_error.IsDefined();
-	}
-
-	/** Retrieve init error if state not ok */
-	const Error &GetInitError() const {
-		return init_error;
-	}
-
-	UpnpClient_Handle getclh()
-	{
-		return m_clh;
-	}
-};
-
-#endif /* _LIBUPNP.H_X_INCLUDED_ */
+#endif