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.

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

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;
 

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;
+static Mutex upnp_client_init_mutex;
+static unsigned upnp_client_ref;
+static UpnpClient_Handle upnp_client_handle;
 
-	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;
-	}
-}
-
-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();
 }

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