diff --git a/include/mapnik/datasource_cache.hpp b/include/mapnik/datasource_cache.hpp index bf951e7f7..4f90f8907 100644 --- a/include/mapnik/datasource_cache.hpp +++ b/include/mapnik/datasource_cache.hpp @@ -33,6 +33,7 @@ #include #include #include +#include namespace mapnik { @@ -56,6 +57,11 @@ private: ~datasource_cache(); std::map > plugins_; std::set plugin_directories_; + // the singleton has a mutex protecting the instance pointer, + // but the instance also needs its own mutex to protect the + // plugins_ and plugin_directories_ members which are potentially + // modified recusrively by register_datasources(path, true); + std::recursive_mutex instance_mutex_; }; extern template class MAPNIK_DECL singleton; diff --git a/src/datasource_cache.cpp b/src/datasource_cache.cpp index eb2cfd0d7..f4124460c 100644 --- a/src/datasource_cache.cpp +++ b/src/datasource_cache.cpp @@ -88,7 +88,7 @@ datasource_ptr datasource_cache::create(parameters const& params) // add scope to ensure lock is released asap { #ifdef MAPNIK_THREADSAFE - std::lock_guard lock(mutex_); + std::lock_guard lock(instance_mutex_); #endif itr=plugins_.find(*type); if (itr == plugins_.end()) @@ -132,6 +132,9 @@ datasource_ptr datasource_cache::create(parameters const& params) std::string datasource_cache::plugin_directories() { +#ifdef MAPNIK_THREADSAFE + std::lock_guard lock(instance_mutex_); +#endif return boost::algorithm::join(plugin_directories_,", "); } @@ -143,6 +146,10 @@ std::vector datasource_cache::plugin_names() names = get_static_datasource_names(); #endif +#ifdef MAPNIK_THREADSAFE + std::lock_guard lock(instance_mutex_); +#endif + std::map >::const_iterator itr; for (itr = plugins_.begin(); itr != plugins_.end(); ++itr) { @@ -155,7 +162,7 @@ std::vector datasource_cache::plugin_names() bool datasource_cache::register_datasources(std::string const& dir, bool recurse) { #ifdef MAPNIK_THREADSAFE - std::lock_guard lock(mutex_); + std::lock_guard lock(instance_mutex_); #endif if (!mapnik::util::exists(dir)) { @@ -202,6 +209,9 @@ bool datasource_cache::register_datasources(std::string const& dir, bool recurse bool datasource_cache::register_datasource(std::string const& filename) { +#ifdef MAPNIK_THREADSAFE + std::lock_guard lock(instance_mutex_); +#endif try { if (!mapnik::util::exists(filename)) diff --git a/test/standalone/datasource_registration_test.cpp b/test/standalone/datasource_registration_test.cpp new file mode 100644 index 000000000..2edbcd72f --- /dev/null +++ b/test/standalone/datasource_registration_test.cpp @@ -0,0 +1,46 @@ +#define CATCH_CONFIG_MAIN +#include "catch.hpp" + +#include +#include + +#include +#include +#include + +TEST_CASE("datasource_cache") { + +SECTION("registration") { + try + { + mapnik::logger logger; + mapnik::logger::severity_type original_severity = logger.get_severity(); + bool success = false; + auto &cache = mapnik::datasource_cache::instance(); + + // registering a directory without any plugins should return false + success = cache.register_datasources("test/data/vrt"); + CHECK(success == false); + + // registering a directory for the first time should return true + success = cache.register_datasources("plugins/input"); + REQUIRE(success == true); + + // registering the same directory again should now return false + success = cache.register_datasources("plugins/input"); + CHECK(success == false); + + // registering the same directory again, but recursively should + // still return false - even though there are subdirectories, they + // do not contain any more plugins. + success = cache.register_datasources("plugins/input", true); + CHECK(success == false); + } + catch (std::exception const & ex) + { + std::clog << ex.what() << "\n"; + REQUIRE(false); + } + +} +}