Merge pull request #4279 from mathisloge/fix/issue-4268

fix #4268 by adding mapped_memory_file

include/mapnik/mapped_memory_cache.hpp

@@ -54,6 +54,14 @@ class MAPNIK_DECL mapped_memory_cache :
     std::unordered_map<std::string,mapped_region_ptr> cache_;
 public:
     bool insert(std::string const& key, mapped_region_ptr);
+    /**
+     * @brief removes the resource identified by key from the cache, if exists
+     * 
+     * @param key unique identifier for the resource
+     * @return true if the resource was removed
+     * @return false if the resource was not removed or wasn't in the cache
+     */
+    bool remove(std::string const& key);
     boost::optional<mapped_region_ptr> find(std::string const& key, bool update_cache = false);
     void clear();
 };

include/mapnik/util/mapped_memory_file.hpp

@@ -0,0 +1,78 @@
+/*****************************************************************************
+ *
+ * This file is part of Mapnik (c++ mapping toolkit)
+ *
+ * Copyright (C) 2021 Artem Pavlenko
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ *****************************************************************************/
+
+#ifndef MAPNIK_MEMORY_MAPPED_FILE_HPP
+#define MAPNIK_MEMORY_MAPPED_FILE_HPP
+
+#include <mapnik/config.hpp>
+#include <mapnik/util/noncopyable.hpp>
+
+#if defined(MAPNIK_MEMORY_MAPPED_FILE)
+#include <boost/interprocess/mapped_region.hpp>
+#include <boost/interprocess/streams/bufferstream.hpp>
+#include <mapnik/mapped_memory_cache.hpp>
+#else
+#include <fstream>
+#endif
+
+#include <string>
+
+namespace mapnik { namespace util {
+
+/**
+ * @brief memory mapped file abstraction. Implementation depends on MAPNIK_MEMORY_MAPPED_FILE. 
+ * Might be a simple file wrapper, if MAPNIK_MEMORY_MAPPED_FILE=0
+ * 
+ */
+class MAPNIK_DECL mapped_memory_file : public noncopyable {
+    public:
+#ifdef MAPNIK_MEMORY_MAPPED_FILE
+        using file_source_type = boost::interprocess::ibufferstream;
+#else
+        using file_source_type = std::ifstream;
+#endif
+
+    public:
+        mapped_memory_file();
+        explicit mapped_memory_file(std::string const& file_name);
+        virtual ~mapped_memory_file();
+
+        file_source_type& file();
+        bool is_open() const;
+        void skip(std::streampos bytes);
+
+        /**
+         * @brief deletes the file identified by file_name. Might also remove the file from any caches.
+         */
+        static void deleteFile(std::string const& file_name);
+    protected:
+        const std::string file_name_;
+#ifdef MAPNIK_MEMORY_MAPPED_FILE
+        mapnik::mapped_region_ptr mapped_region_;
+#endif
+        file_source_type file_;
+};
+
+
+} } 
+
+#endif

plugins/input/csv/csv_datasource.cpp

@@ -53,6 +53,7 @@ MAPNIK_DISABLE_WARNING_PUSH
 MAPNIK_DISABLE_WARNING_POP
 #include <mapnik/mapped_memory_cache.hpp>
 #endif
+#include <mapnik/util/mapped_memory_file.hpp>
 
 // stl
 #include <sstream>
@@ -120,35 +121,8 @@ csv_datasource::csv_datasource(parameters const& params)
     }
     else
     {
-#if defined (MAPNIK_MEMORY_MAPPED_FILE)
+        mapnik::util::mapped_memory_file in_file{filename_};
-        using file_source_type = boost::interprocess::ibufferstream;
+        parse_csv(in_file.file());
-        file_source_type in;
-        mapnik::mapped_region_ptr mapped_region;
-        boost::optional<mapnik::mapped_region_ptr> memory =
-            mapnik::mapped_memory_cache::instance().find(filename_, true);
-        if (memory)
-        {
-            mapped_region = *memory;
-            in.buffer(static_cast<char*>(mapped_region->get_address()),mapped_region->get_size());
-        }
-        else
-        {
-            throw std::runtime_error("could not create file mapping for " + filename_);
-        }
-#elif defined (_WIN32)
-        std::ifstream in(mapnik::utf8_to_utf16(filename_),std::ios_base::in | std::ios_base::binary);
-        if (!in.is_open())
-        {
-            throw mapnik::datasource_exception("CSV Plugin: could not open: '" + filename_ + "'");
-        }
-#else
-        std::ifstream in(filename_.c_str(),std::ios_base::in | std::ios_base::binary);
-        if (!in.is_open())
-        {
-            throw mapnik::datasource_exception("CSV Plugin: could not open: '" + filename_ + "'");
-        }
-#endif
-        parse_csv(in);
 
         if (has_disk_index_ && !extent_initialized_)
         {

plugins/input/shape/dbfile.cpp

@@ -51,31 +51,12 @@ dbf_file::dbf_file()
       record_(0) {}
 
 dbf_file::dbf_file(std::string const& file_name)
-    :num_records_(0),
+    :mapped_memory_file{file_name},
+     num_records_(0),
      num_fields_(0),
      record_length_(0),
-#if defined(MAPNIK_MEMORY_MAPPED_FILE)
-     file_(),
-#elif defined(_WIN32)
-     file_(mapnik::utf8_to_utf16(file_name), std::ios::in | std::ios::binary),
-#else
-     file_(file_name.c_str() ,std::ios::in | std::ios::binary),
-#endif
      record_(0)
 {
-
-#if defined(MAPNIK_MEMORY_MAPPED_FILE)
-    boost::optional<mapnik::mapped_region_ptr> memory = mapnik::mapped_memory_cache::instance().find(file_name,true);
-    if (memory)
-    {
-        mapped_region_ = *memory;
-        file_.buffer(static_cast<char*>((*memory)->get_address()),(*memory)->get_size());
-    }
-    else
-    {
-        throw std::runtime_error("could not create file mapping for "+file_name);
-    }
-#endif
     if (file_)
     {
         read_header();
@@ -88,16 +69,6 @@ dbf_file::~dbf_file()
     ::operator delete(record_);
 }
 
-
-bool dbf_file::is_open()
-{
-#if defined(MAPNIK_MEMORY_MAPPED_FILE)
-    return (file_.buffer().second > 0);
-#else
-    return file_.is_open();
-#endif
-}
-
 int dbf_file::num_records() const
 {
     return num_records_;
@@ -274,8 +245,3 @@ int dbf_file::read_int()
     mapnik::read_int32_ndr(b,val);
     return val;
 }
-
-void dbf_file::skip(int bytes)
-{
-    file_.seekg(bytes,std::ios::cur);
-}

plugins/input/shape/dbfile.hpp

@@ -28,14 +28,7 @@
 #include <mapnik/util/noncopyable.hpp>
 #include <mapnik/unicode.hpp>
 
-#if defined(MAPNIK_MEMORY_MAPPED_FILE)
+#include <mapnik/util/mapped_memory_file.hpp>
-#include <mapnik/mapped_memory_cache.hpp>
-#include <mapnik/warning.hpp>
-MAPNIK_DISABLE_WARNING_PUSH
-#include <mapnik/warning_ignore.hpp>
-#include <boost/interprocess/streams/bufferstream.hpp>
-MAPNIK_DISABLE_WARNING_POP
-#endif
 
 // stl
 #include <vector>
@@ -54,25 +47,18 @@ struct field_descriptor
 };
 
 
-class dbf_file : private mapnik::util::noncopyable
+class dbf_file : public mapnik::util::mapped_memory_file
 {
 private:
     int num_records_;
     int num_fields_;
     std::size_t record_length_;
     std::vector<field_descriptor> fields_;
-#if defined(MAPNIK_MEMORY_MAPPED_FILE)
-    boost::interprocess::ibufferstream file_;
-    mapnik::mapped_region_ptr mapped_region_;
-#else
-    std::ifstream file_;
-#endif
     char* record_;
 public:
     dbf_file();
     dbf_file(std::string const& file_name);
     ~dbf_file();
-    bool is_open();
     int num_records() const;
     int num_fields() const;
     field_descriptor const& descriptor(int col) const;
@@ -83,7 +69,6 @@ private:
     void read_header();
     int read_short();
     int read_int();
-    void skip(int bytes);
 };
 
 #endif //DBFFILE_HPP

plugins/input/shape/shapefile.hpp

@@ -35,16 +35,12 @@
 #include <mapnik/geometry/box2d.hpp>
 
 #if defined(MAPNIK_MEMORY_MAPPED_FILE)
-#include <mapnik/warning.hpp>
-MAPNIK_DISABLE_WARNING_PUSH
-#include <mapnik/warning_ignore.hpp>
 #include <boost/interprocess/mapped_region.hpp>
-#include <boost/interprocess/streams/bufferstream.hpp>
-MAPNIK_DISABLE_WARNING_POP
-#include <mapnik/mapped_memory_cache.hpp>
 #endif
 #include <mapnik/util/noncopyable.hpp>
 
+#include <mapnik/util/mapped_memory_file.hpp>
+
 using mapnik::box2d;
 using mapnik::read_int32_ndr;
 using mapnik::read_int32_xdr;
@@ -143,64 +139,24 @@ struct shape_record
     std::size_t length() {return size;}
 };
 
-class shape_file : mapnik::util::noncopyable
+class shape_file : public mapnik::util::mapped_memory_file
 {
 public:
-
 #if defined(MAPNIK_MEMORY_MAPPED_FILE)
-    using file_source_type = boost::interprocess::ibufferstream;
     using record_type = shape_record<MappedRecordTag>;
-    mapnik::mapped_region_ptr mapped_region_;
 #else
-    using file_source_type = std::ifstream;
     using record_type = shape_record<RecordTag>;
 #endif
 
-    file_source_type file_;
-
     shape_file() {}
 
-    shape_file(std::string  const& file_name) :
+    shape_file(std::string  const& file_name)
-#if defined(MAPNIK_MEMORY_MAPPED_FILE)
+      : mapped_memory_file(file_name)
-        file_()
-#elif defined(_WIN32)
-        file_(mapnik::utf8_to_utf16(file_name), std::ios::in | std::ios::binary)
-#else
-        file_(file_name.c_str(), std::ios::in | std::ios::binary)
-#endif
     {
-#if defined(MAPNIK_MEMORY_MAPPED_FILE)
-        boost::optional<mapnik::mapped_region_ptr> memory =
-            mapnik::mapped_memory_cache::instance().find(file_name,true);
-
-        if (memory)
-        {
-            mapped_region_ = *memory;
-            file_.buffer(static_cast<char*>(mapped_region_->get_address()),mapped_region_->get_size());
-        }
-        else
-        {
-            throw std::runtime_error("could not create file mapping for "+file_name);
-        }
-#endif
     }
 
     ~shape_file() {}
 
-    inline file_source_type& file()
-    {
-        return file_;
-    }
-
-    inline bool is_open() const
-    {
-#if defined(MAPNIK_MEMORY_MAPPED_FILE)
-        return (file_.buffer().second > 0);
-#else
-        return file_.is_open();
-#endif
-    }
-
     inline void read_record(record_type& rec)
     {
 #if defined(MAPNIK_MEMORY_MAPPED_FILE)
@@ -241,11 +197,6 @@ public:
         file_.read(reinterpret_cast<char*>(&envelope), sizeof(envelope));
     }
 
-    inline void skip(std::streampos bytes)
-    {
-        file_.seekg(bytes, std::ios::cur);
-    }
-
     inline void rewind()
     {
         seek(100);

src/CMakeLists.txt

@@ -223,6 +223,7 @@ target_sources(mapnik PRIVATE
 target_sources(mapnik PRIVATE
     util/math.cpp
     util/utf_conv_win.cpp
+    util/mapped_memory_file.cpp
 )
 
 if(USE_CAIRO)

src/build.py

@@ -274,6 +274,7 @@ source = Split(
     renderer_common/render_thunk_extractor.cpp
     renderer_common/pattern_alignment.cpp
     util/math.cpp
+    util/mapped_memory_file.cpp
     value.cpp
     """
     )

src/mapped_memory_cache.cpp

@@ -56,6 +56,14 @@ bool mapped_memory_cache::insert(std::string const& uri, mapped_region_ptr mem)
     return cache_.emplace(uri,mem).second;
 }
 
+bool mapped_memory_cache::remove(std::string const& key)
+{
+#ifdef MAPNIK_THREADSAFE
+    std::lock_guard<std::mutex> lock(mutex_);
+#endif
+    return cache_.erase(key) > 0;
+}
+
 boost::optional<mapped_region_ptr> mapped_memory_cache::find(std::string const& uri, bool update_cache)
 {
 #ifdef MAPNIK_THREADSAFE

src/util/mapped_memory_file.cpp

@@ -0,0 +1,76 @@
+#include <mapnik/mapped_memory_cache.hpp>
+#include <mapnik/util/mapped_memory_file.hpp>
+#include <mapnik/util/fs.hpp>
+#ifdef _WIN32
+#include <mapnik/util/utf_conv_win.hpp>
+#endif
+
+namespace mapnik
+{
+    namespace util
+    {
+        mapped_memory_file::mapped_memory_file() {}
+        
+        mapped_memory_file::mapped_memory_file(std::string const &file_name)
+            : file_name_{file_name},
+#if defined(MAPNIK_MEMORY_MAPPED_FILE)
+              file_()
+#elif defined(_WIN32)
+              file_(mapnik::utf8_to_utf16(file_name), std::ios::in | std::ios::binary)
+#else
+              file_(file_name.c_str(), std::ios::in | std::ios::binary)
+#endif
+        {
+#if defined(MAPNIK_MEMORY_MAPPED_FILE)
+        boost::optional<mapnik::mapped_region_ptr> memory =
+            mapnik::mapped_memory_cache::instance().find(file_name, true);
+
+        if (memory)
+        {
+            mapped_region_ = *memory;
+            file_.buffer(static_cast<char*>(mapped_region_->get_address()),mapped_region_->get_size());
+        }
+        else
+        {
+            throw std::runtime_error("could not create file mapping for "+file_name);
+        }
+#endif
+
+        }
+
+        mapped_memory_file::file_source_type& mapped_memory_file::file()
+        {
+            return file_;
+        }
+
+        bool mapped_memory_file::is_open() const
+        {
+    #if defined(MAPNIK_MEMORY_MAPPED_FILE)
+            return (file_.buffer().second > 0);
+    #else
+            return file_.is_open();
+    #endif
+        }
+
+        void mapped_memory_file::skip(std::streampos bytes)
+        {
+            file_.seekg(bytes, std::ios::cur);
+        }
+
+        void mapped_memory_file::deleteFile(std::string const &file_name)
+        {
+#ifdef MAPNIK_MEMORY_MAPPED_FILE
+            mapped_memory_cache::instance().remove(file_name);
+#endif
+            if(util::exists(file_name)) {
+                util::remove(file_name);
+            }
+            
+        }
+
+        mapped_memory_file::~mapped_memory_file()
+        {
+        }
+
+    }
+}

test/unit/datasource/shapeindex.cpp

@@ -33,6 +33,7 @@ MAPNIK_DISABLE_WARNING_PUSH
 #include <mapnik/warning_ignore.hpp>
 #include <boost/algorithm/string.hpp>
 MAPNIK_DISABLE_WARNING_POP
+#include <mapnik/util/mapped_memory_file.hpp>
 
 namespace {
 
@@ -107,10 +108,7 @@ TEST_CASE("invalid shapeindex")
                 std::string path = "test/data/shp/boundaries.shp";
                 std::string index_path = path.substr(0, path.rfind(".")) + ".index";
                 // remove *.index if present
-                if (mapnik::util::exists(index_path))
+                mapnik::util::mapped_memory_file::deleteFile(index_path);
-                {
-                    mapnik::util::remove(index_path);
-                }
                 // count features
 
                 std::size_t feature_count = count_shapefile_features(path);
@@ -132,10 +130,7 @@ TEST_CASE("invalid shapeindex")
                     CHECK(feature_count_indexed == 0);
                 }
                 // remove *.index if present
-                if (mapnik::util::exists(index_path))
+                mapnik::util::mapped_memory_file::deleteFile(index_path);
-                {
-                    mapnik::util::remove(index_path);
-                }
             }
         }
     }
@@ -159,10 +154,7 @@ TEST_CASE("shapeindex")
 
                         std::string index_path = path.substr(0, path.rfind(".")) + ".index";
                         // remove *.index if present
-                        if (mapnik::util::exists(index_path))
+                        mapnik::util::mapped_memory_file::deleteFile(index_path);
-                        {
-                            mapnik::util::remove(index_path);
-                        }
                         // count features
                         std::size_t feature_count = count_shapefile_features(path);
                         // create *.index
@@ -180,10 +172,7 @@ TEST_CASE("shapeindex")
                         // ensure number of features are the same
                         REQUIRE(feature_count == feature_count_indexed);
                         // remove *.index if present
-                        if (mapnik::util::exists(index_path))
+                        mapnik::util::mapped_memory_file::deleteFile(index_path);
-                        {
-                            mapnik::util::remove(index_path);
-                        }
                     }
                 }
             }

test/unit/imaging/image_io_test.cpp

@@ -19,6 +19,7 @@ MAPNIK_DISABLE_WARNING_PUSH
 #include <boost/filesystem/convenience.hpp>
 #include <boost/optional/optional_io.hpp>
 MAPNIK_DISABLE_WARNING_POP
+#include <mapnik/util/mapped_memory_file.hpp>
 
 inline void make_directory(std::string const& dir) {
     boost::filesystem::create_directories(dir);
@@ -205,10 +206,7 @@ SECTION("image_util : save_to_file/save_to_stream/save_to_string")
                 CHECK(0 == std::memcmp(im2.bytes(), im.bytes(), im.width() * im.height()));
             }
         }
-        if (mapnik::util::exists(filename))
+        mapnik::util::mapped_memory_file::deleteFile(filename);
-        {
-            mapnik::util::remove(filename);
-        }
     }
 }
 

utils/mapnik-index/process_csv_file.cpp

@@ -28,15 +28,9 @@
 #include <mapnik/util/utf_conv_win.hpp>
 
 #if defined(MAPNIK_MEMORY_MAPPED_FILE)
-#include <mapnik/warning.hpp>
-MAPNIK_DISABLE_WARNING_PUSH
-#include <mapnik/warning_ignore.hpp>
 #include <boost/algorithm/string.hpp>
-#include <boost/interprocess/mapped_region.hpp>
-#include <boost/interprocess/streams/bufferstream.hpp>
-MAPNIK_DISABLE_WARNING_POP
-#include <mapnik/mapped_memory_cache.hpp>
 #endif
+#include <mapnik/util/mapped_memory_file.hpp>
 
 #include <fstream>
 #include <iostream>
@@ -53,37 +47,11 @@ std::pair<bool,typename T::value_type::first_type> process_csv_file(T & boxes, s
     p.separator_ = separator;
     p.quote_ = quote;
 
-#if defined(MAPNIK_MEMORY_MAPPED_FILE)
+
-    using file_source_type = boost::interprocess::ibufferstream;
+    util::mapped_memory_file csv_file{filename};
-    file_source_type csv_file;
-    mapnik::mapped_region_ptr mapped_region;
-    boost::optional<mapnik::mapped_region_ptr> memory =
-        mapnik::mapped_memory_cache::instance().find(filename, true);
-    if (memory)
-    {
-        mapped_region = *memory;
-        csv_file.buffer(static_cast<char*>(mapped_region->get_address()),mapped_region->get_size());
-    }
-    else
-    {
-        std::clog << "Error : cannot mmap " << filename << std::endl;
-        return std::make_pair(false, box_type(p.extent_));
-    }
-#else
- #if defined(_WIN32)
-    std::ifstream csv_file(mapnik::utf8_to_utf16(filename),std::ios_base::in | std::ios_base::binary);
- #else
-    std::ifstream csv_file(filename.c_str(),std::ios_base::in | std::ios_base::binary);
- #endif
-    if (!csv_file.is_open())
-    {
-        std::clog << "Error : cannot open " << filename << std::endl;
-        return std::make_pair(false, box_type(p.extent_));
-    }
-#endif
     try
     {
-        p.parse_csv_and_boxes(csv_file, boxes);
+        p.parse_csv_and_boxes(csv_file.file(), boxes);
         return std::make_pair(true, box_type(p.extent_));
     }
     catch (std::exception const& ex)