Merge pull request #2688 from ziollek/lmdb_single_env

LMDB - fix integration, restoring ability of use lmdb with nginx-modsecurity

src/collection/backend/lmdb.cc

@@ -22,6 +22,8 @@
 #include <string>
 #include <memory>
 
+#include <pthread.h>
+
 #include "modsecurity/variable_value.h"
 #include "src/utils/regex.h"
 #include "src/variables/variable.h"
@@ -36,22 +38,17 @@ namespace backend {
 #ifdef WITH_LMDB
 
 LMDB::LMDB(std::string name) :
-    Collection(name), m_env(NULL) {
+    Collection(name), m_env(NULL), isOpen(false) {}
-    MDB_txn *txn;
+
-    mdb_env_create(&m_env);
+int LMDB::txn_begin(unsigned int flags, MDB_txn **ret) {
-    mdb_env_open(m_env, "./modsec-shared-collections",
+    if (!isOpen) {
-        MDB_WRITEMAP | MDB_NOSUBDIR, 0664);
+        m_env = MDBEnvProvider::GetInstance().GetEnv();
-    mdb_txn_begin(m_env, NULL, 0, &txn);
+        m_dbi = *(MDBEnvProvider::GetInstance().GetDBI());
-    mdb_dbi_open(txn, NULL, MDB_CREATE | MDB_DUPSORT, &m_dbi);
+        isOpen = true;
-    mdb_txn_commit(txn);
+    }
+    return mdb_txn_begin(m_env, NULL, flags, ret);
 }
 
-
-LMDB::~LMDB() {
-    mdb_env_close(m_env);
-}
-
-
 void LMDB::string2val(const std::string& str, MDB_val *val) {
     val->mv_size = sizeof(char)*(str.size());
     val->mv_data = const_cast<char *>(str.c_str());
@@ -159,7 +156,7 @@ std::unique_ptr<std::string> LMDB::resolveFirst(const std::string& var) {
 
     string2val(var, &mdb_key);
 
-    rc = mdb_txn_begin(m_env, NULL, MDB_RDONLY, &txn);
+    rc = txn_begin(MDB_RDONLY, &txn);
     lmdb_debug(rc, "txn", "resolveFirst");
     if (rc != 0) {
         goto end_txn;
@@ -192,7 +189,7 @@ bool LMDB::storeOrUpdateFirst(const std::string &key,
     string2val(key, &mdb_key);
     string2val(value, &mdb_value);
 
-    rc = mdb_txn_begin(m_env, NULL, 0, &txn);
+    rc = txn_begin(0, &txn);
     lmdb_debug(rc, "txn", "storeOrUpdateFirst");
     if (rc != 0) {
         goto end_txn;
@@ -240,7 +237,7 @@ void LMDB::resolveSingleMatch(const std::string& var,
     MDB_val mdb_value_ret;
     MDB_cursor *cursor;
 
-    rc = mdb_txn_begin(m_env, NULL, MDB_RDONLY, &txn);
+    rc = txn_begin(MDB_RDONLY, &txn);
     lmdb_debug(rc, "txn", "resolveSingleMatch");
     if (rc != 0) {
         goto end_txn;
@@ -271,7 +268,7 @@ void LMDB::store(std::string key, std::string value) {
     int rc;
     MDB_stat mst;
 
-    rc = mdb_txn_begin(m_env, NULL, 0, &txn);
+    rc = txn_begin(0, &txn);
     lmdb_debug(rc, "txn", "store");
     if (rc != 0) {
         goto end_txn;
@@ -310,7 +307,7 @@ bool LMDB::updateFirst(const std::string &key,
     MDB_val mdb_value;
     MDB_val mdb_value_ret;
 
-    rc = mdb_txn_begin(m_env, NULL, 0, &txn);
+    rc = txn_begin(0, &txn);
     lmdb_debug(rc, "txn", "updateFirst");
     if (rc != 0) {
         goto end_txn;
@@ -364,7 +361,7 @@ void LMDB::del(const std::string& key) {
     MDB_val mdb_value_ret;
     MDB_stat mst;
 
-    rc = mdb_txn_begin(m_env, NULL, 0, &txn);
+    rc = txn_begin(0, &txn);
     lmdb_debug(rc, "txn", "del");
     if (rc != 0) {
         goto end_txn;
@@ -411,7 +408,7 @@ void LMDB::resolveMultiMatches(const std::string& var,
     size_t keySize = var.size();
     MDB_cursor *cursor;
 
-    rc = mdb_txn_begin(m_env, NULL, MDB_RDONLY, &txn);
+    rc = txn_begin(MDB_RDONLY, &txn);
     lmdb_debug(rc, "txn", "resolveMultiMatches");
     if (rc != 0) {
         goto end_txn;
@@ -465,7 +462,7 @@ void LMDB::resolveRegularExpression(const std::string& var,
 
     Utils::Regex r(var, true);
 
-    rc = mdb_txn_begin(m_env, NULL, MDB_RDONLY, &txn);
+    rc = txn_begin(MDB_RDONLY, &txn);
     lmdb_debug(rc, "txn", "resolveRegularExpression");
     if (rc != 0) {
         goto end_txn;
@@ -503,6 +500,30 @@ end_txn:
     return;
 }
 
+
+MDBEnvProvider::MDBEnvProvider() : m_env(NULL) {
+    MDB_txn *txn;
+    mdb_env_create(&m_env);
+    mdb_env_open(m_env, "./modsec-shared-collections",
+        MDB_WRITEMAP | MDB_NOSUBDIR, 0664);
+    mdb_txn_begin(m_env, NULL, 0, &txn);
+    mdb_dbi_open(txn, NULL, MDB_CREATE | MDB_DUPSORT, &m_dbi);
+    mdb_txn_commit(txn);
+}
+
+MDB_env* MDBEnvProvider::GetEnv() {
+    return m_env;
+}
+
+MDB_dbi* MDBEnvProvider::GetDBI() {
+    return &m_dbi;
+}
+
+MDBEnvProvider::~MDBEnvProvider() {
+    mdb_dbi_close(m_env, m_dbi);
+    mdb_env_close(m_env);
+}
+
 #endif
 
 }  // namespace backend

src/collection/backend/lmdb.h

@@ -48,11 +48,53 @@ namespace modsecurity {
 namespace collection {
 namespace backend {
 
+
+/**
+ * The MDBEnvProvider class defines the `GetInstance` method that serves as an
+ * alternative to constructor and lets clients access the same instance of this
+ * class over and over. Its used to provide single MDB_env instance for each collection
+ * that uses lmdb to store and retrieve data. That approach satisfies lmdb requirement:
+ *
+ *   "LMDB uses POSIX locks on files, and these locks have issues if one process opens
+ *    a file multiple times. Because of this, do not mdb_env_open() a file multiple
+ *    times from a single process."
+ *
+ * Creation of MDB_env is delayed to moment when first transaction is opened.
+ * This approach prevents passing env object to forked processes.
+ * In that way next lmdb requirement be satisfied:
+ *
+ *   "Use an MDB_env* in the process which opened it, without fork()ing."
+ */
+class MDBEnvProvider {
+
+ public:
+    MDBEnvProvider(MDBEnvProvider &other) = delete;
+    void operator=(const MDBEnvProvider &) = delete;
+
+    /**
+     * This is the static method that controls the access to the singleton
+     * instance. On the first run, it creates a singleton object and places it
+     * into the static field. On subsequent runs, it returns the client existing
+     * object stored in the static field (Meyers Singleton implementation).
+     */
+    static MDBEnvProvider& GetInstance() {
+        static MDBEnvProvider instance;
+        return instance;
+    }
+    MDB_env* GetEnv();
+    MDB_dbi* GetDBI();
+    ~MDBEnvProvider();
+ private:
+    MDB_env *m_env;
+    MDB_dbi m_dbi;
+
+    MDBEnvProvider();
+};
+
 class LMDB :
     public Collection {
  public:
     explicit LMDB(std::string name);
-    ~LMDB();
     void store(std::string key, std::string value) override;
 
     bool storeOrUpdateFirst(const std::string &key,
@@ -75,11 +117,13 @@ class LMDB :
         variables::KeyExclusions &ke) override;
 
  private:
+    int txn_begin(unsigned int flags, MDB_txn **ret);
     void string2val(const std::string& str, MDB_val *val);
     void inline lmdb_debug(int rc, std::string op, std::string scope);
 
     MDB_env *m_env;
     MDB_dbi m_dbi;
+    bool isOpen;
 };
 
 }  // namespace backend