Issue 6494 - Various errors when using extended matching rule on vlv sort filter

dirsrvtests/tests/suites/indexes/regression_test.py

@@ -546,6 +546,35 @@ def test_task_and_be(topo, add_backend_and_ldif_50K_users):
     assert user.get_attr_val_utf8_l('description') == descval
 
 
+def test_reindex_extended_matching_rule(topo, add_backend_and_ldif_50K_users):
+    """Check that index with extended matching rule are reindexed properly.
+
+    :id: 8a3198e8-cc5a-11ef-a3e7-482ae39447e5
+    :setup: Standalone instance + a second backend with 50K users
+    :steps:
+        1. Configure uid with 2.5.13.2 matching rule
+        1. Configure cn with 2.5.13.2 matching rule
+        2. Reindex
+    :expectedresults:
+        1. Success
+        2. Success
+    """
+
+    inst = topo.standalone
+    tasks = Tasks(inst)
+    be2 = Backends(topo.standalone).get_backend(SUFFIX2)
+    index = be2.get_index('uid')
+    index.replace('nsMatchingRule', '2.5.13.2')
+    index = be2.get_index('cn')
+    index.replace('nsMatchingRule', '2.5.13.2')
+
+    assert tasks.reindex(
+        suffix=SUFFIX2,
+        args={TASK_WAIT: True}
+    ) == 0
+
+
+
 if __name__ == "__main__":
     # Run isolated
     # -s for DEBUG mode

dirsrvtests/tests/suites/vlv/regression_test.py

@@ -142,7 +142,7 @@ def add_users(inst, users_num, suffix=DEFAULT_SUFFIX):
 
 
 def create_vlv_search_and_index(inst, basedn=DEFAULT_SUFFIX, bename='userRoot',
-                                scope=ldap.SCOPE_SUBTREE, prefix="vlv"):
+                                scope=ldap.SCOPE_SUBTREE, prefix="vlv", vlvsort="cn"):
     vlv_searches = VLVSearch(inst)
     vlv_search_properties = {
         "objectclass": ["top", "vlvSearch"],
@@ -160,7 +160,7 @@ def create_vlv_search_and_index(inst, basedn=DEFAULT_SUFFIX, bename='userRoot',
     vlv_index_properties = {
         "objectclass": ["top", "vlvIndex"],
         "cn": f"{prefix}Idx",
-        "vlvsort": "cn",
+        "vlvsort": vlvsort,
     }
     vlv_index.create(
         basedn=f"cn={prefix}Srch,cn={bename},cn=ldbm database,cn=plugins,cn=config",
@@ -312,6 +312,40 @@ def fin():
     return topology_st
 
 
+@pytest.fixture
+def vlv_setup_with_uid_mr(topology_st, request):
+    inst = topology_st.standalone
+    bename = 'be1'
+    besuffix = f'o={bename}'
+    beh = BackendHandler(inst, { bename: besuffix })
+
+    def fin():
+        # Cleanup function
+        if not DEBUGGING and inst.exists() and inst.status():
+            beh.cleanup()
+
+    request.addfinalizer(fin)
+
+    # Make sure that our backend are not already present.
+    beh.cleanup()
+
+    # Then add the new backend
+    beh.setup()
+
+    index = Index(inst, f'cn=uid,cn=index,cn={bename},cn=ldbm database,cn=plugins,cn=config')
+    index.add('nsMatchingRule', '2.5.13.2')
+    reindex_task = Tasks(inst)
+    assert reindex_task.reindex(
+        suffix=besuffix,
+        attrname='uid',
+        args={TASK_WAIT: True}
+    ) == 0
+
+    topology_st.beh = beh
+    return topology_st
+
+
+
 @pytest.fixture
 def freeipa(topology_st):
     # generate a standalone instance with same vlv config than freeipa
@@ -1087,6 +1121,50 @@ def test_vlv_logs(vlv_setup_nested_backends):
     assert 'err=0 ' in res
 
 
+def test_vlv_with_mr(vlv_setup_with_uid_mr):
+    """
+    Testing vlv having specific matching rule
+
+    :id: 5e04afe2-beec-11ef-aa84-482ae39447e5
+    :setup: Standalone with uid have a matching rule index
+    :steps:
+        1. Append vlvIndex entries then vlvSearch entry in the dse.ldif
+        2. Restart the server
+    :expectedresults:
+        1. Should Success.
+        2. Should Success.
+    """
+    inst = vlv_setup_with_uid_mr.standalone
+    beh = vlv_setup_with_uid_mr.beh
+    bename, besuffix = next(iter(beh.bedict.items()))
+    vlv_searches, vlv_index = create_vlv_search_and_index(
+                                inst, basedn=besuffix, bename=bename,
+                                vlvsort="uid:2.5.13.2")
+    # Reindex the vlv
+    reindex_task = Tasks(inst)
+    assert reindex_task.reindex(
+        suffix=besuffix,
+        attrname=vlv_index.rdn,
+        args={TASK_WAIT: True},
+        vlv=True
+    ) == 0
+
+    inst.restart()
+    users = UserAccounts(inst, besuffix)
+    user_properties = {
+        'uid': f'a new testuser',
+        'cn': f'a new testuser',
+        'sn': 'user',
+        'uidNumber': '0',
+        'gidNumber': '0',
+        'homeDirectory': 'foo'
+    }
+    user = users.create(properties=user_properties)
+    user.delete()
+    assert inst.status()
+
+
+
 if __name__ == "__main__":
     # Run isolated
     # -s for DEBUG mode

ldap/servers/slapd/back-ldbm/cleanup.c

@@ -15,12 +15,14 @@
 
 #include "back-ldbm.h"
 #include "dblayer.h"
+#include "vlv_srch.h"
 
 int
 ldbm_back_cleanup(Slapi_PBlock *pb)
 {
     struct ldbminfo *li;
     Slapi_Backend *be;
+    struct vlvSearch *nextp;
 
     slapi_log_err(SLAPI_LOG_TRACE, "ldbm_back_cleanup", "ldbm backend cleaning up\n");
     slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &li);
@@ -45,6 +47,12 @@ ldbm_back_cleanup(Slapi_PBlock *pb)
         return 0;
     }
 
+    /* Release the vlv list */
+    for (struct vlvSearch *p=be->vlvSearchList; p; p=nextp) {
+        nextp = p->vlv_next;
+        vlvSearch_delete(&p);
+    }
+
     /*
      * We check if li is NULL. Because of an issue in how we create backends
      * we share the li and plugin info between many unique backends. This causes

ldap/servers/slapd/back-ldbm/db-mdb/mdb_import_threads.c

@@ -152,6 +152,9 @@ static void dbmdb_import_writeq_push(ImportCtx_t *ctx, WriterQueueData_t *wqd);
 static int have_workers_finished(ImportJob *job);
 struct backentry *dbmdb_import_prepare_worker_entry(WorkerQueueData_t *wqelmnt);
 
+/* Mutex needed for extended matching rules */
+static pthread_mutex_t extended_mr_mutex = PTHREAD_MUTEX_INITIALIZER;
+
 /***************************************************************************/
 /**************************** utility functions ****************************/
 /***************************************************************************/
@@ -3181,6 +3184,23 @@ is_reindexed_attr(const char *attrname, const ImportCtx_t *ctx, char **list)
     return (list && attr_in_list(attrname, list));
 }
 
+/*
+ * Determine if vlv require extended matching rule evaluation
+ */
+static int
+vlv_has_emr(struct vlvIndex *p)
+{
+    if (p->vlv_sortkey != NULL) {
+        /* Foreach sorted attribute... */
+        for (int sortattr = 0; p->vlv_sortkey[sortattr] != NULL; sortattr++) {
+            if (p->vlv_sortkey[sortattr]->sk_matchruleoid != NULL) {
+                return 1;
+            }
+        }
+    }
+    return 0;
+}
+
 static void
 process_vlv_index(backentry *ep, ImportWorkerInfo *info)
 {
@@ -3203,7 +3223,18 @@ process_vlv_index(backentry *ep, ImportWorkerInfo *info)
         slapi_pblock_set(pb, SLAPI_BACKEND, be);
         if (vlv_index && vlv_index->vlv_attrinfo &&
             is_reindexed_attr(vlv_index->vlv_attrinfo->ai_type , ctx, ctx->indexVlvs)) {
-            ret = vlv_update_index(vlv_index, (dbi_txn_t*)&txn, inst->inst_li, pb, NULL, ep);
+            if (vlv_has_emr(vlv_index)) {
+                /*
+                 * Serialize if there is an extended matching rule
+                 * Because matchrule_values_to_keys is not thread safe when indexing
+                 *  because new mr_indexer are created) but that need to be double checked)
+                 */
+                pthread_mutex_lock(&extended_mr_mutex);
+                ret = vlv_update_index(vlv_index, (dbi_txn_t*)&txn, inst->inst_li, pb, NULL, ep);
+                pthread_mutex_unlock(&extended_mr_mutex);
+            } else {
+                ret = vlv_update_index(vlv_index, (dbi_txn_t*)&txn, inst->inst_li, pb, NULL, ep);
+            }
         }
         if (0 != ret) {
             /* Something went wrong, eg disk filled up */

ldap/servers/slapd/back-ldbm/db-mdb/mdb_layer.c

@@ -346,6 +346,7 @@ dbmdb_close(struct ldbminfo *li, int dbmode)
     }
 
     return_value |= dbmdb_post_close(li, dbmode);
+    shutdown_mdbtxn();
 
     return return_value;
 }

ldap/servers/slapd/back-ldbm/db-mdb/mdb_layer.h

@@ -506,6 +506,7 @@ void dbmdb_free_stats(dbmdb_stats_t **stats);
 int dbmdb_reset_vlv_file(backend *be, const char *filename);
 
 /* mdb_txn.c */
+void shutdown_mdbtxn(void);
 int dbmdb_start_txn(const char *funcname, dbi_txn_t *parent_txn, int flags, dbi_txn_t **txn);
 int dbmdb_end_txn(const char *funcname, int rc, dbi_txn_t **txn);
 void init_mdbtxn(dbmdb_ctx_t *ctx);

ldap/servers/slapd/back-ldbm/db-mdb/mdb_txn.c

@@ -41,8 +41,10 @@ cleanup_mdbtxn_stack(void *arg)
     dbmdb_txn_t *txn2;
 
     *anchor = NULL;
+    if (anchor == (dbmdb_txn_t **) PR_GetThreadPrivate(thread_private_mdb_txn_stack)) {
+        PR_SetThreadPrivate(thread_private_mdb_txn_stack, NULL);
+    }
     slapi_ch_free((void**)&anchor);
-    PR_SetThreadPrivate(thread_private_mdb_txn_stack, NULL);
     while (txn) {
         txn2 = txn->parent;
         TXN_ABORT(TXN(txn));
@@ -68,6 +70,14 @@ static dbmdb_txn_t **get_mdbtxnanchor(void)
     return anchor;
 }
 
+void shutdown_mdbtxn(void)
+{
+    dbmdb_txn_t **anchor = (dbmdb_txn_t **) PR_GetThreadPrivate(thread_private_mdb_txn_stack);
+    if (anchor) {
+        PR_SetThreadPrivate(thread_private_mdb_txn_stack, NULL);
+    }
+}
+
 static void push_mdbtxn(dbmdb_txn_t *txn)
 {
     dbmdb_txn_t **anchor = get_mdbtxnanchor();

ldap/servers/slapd/back-ldbm/dblayer.c

@@ -222,7 +222,7 @@
     }
     libpath = backend_implement_get_libpath(li, plgname);
     backend_implement_init = slapi_ch_smprintf("%s_init", plgname);
     backend_implement_init_x = sym_load(libpath, backend_implement_init, "dblayer_implement", 1);
     slapi_ch_free_string(&backend_implement_init);
     if (libpath != li->li_plugin->plg_libpath) {
         slapi_ch_free_string(&libpath);
@@ -385,7 +385,7 @@
 }
 
 int
 dblayer_erase_changelog_file(backend *be, struct attrinfo *a, PRBool use_lock, int no_force_chkpt)
 {
     if ((NULL == be) || (NULL == be->be_database)) {
         return 0;
@@ -494,8 +494,12 @@
 dblayer_close(struct ldbminfo *li, int dbmode)
 {
     dblayer_private *priv = (dblayer_private *)li->li_dblayer_private;
-
-    return priv->dblayer_close_fn(li, dbmode);
+    int rc = priv->dblayer_close_fn(li, dbmode);
+    if (rc == 0) {
+        /* Clean thread specific data */
+        dblayer_destroy_txn_stack();
+    }
+    return rc;
 }
 
 /* Routines for opening and closing random files in the dbi_env_t.
@@ -667,6 +671,9 @@
         return 0;
     }
     struct ldbminfo *li = (struct ldbminfo *)be->be_database->plg_private;
+    if (NULL == li) {
+        return 0;
+    }
     dblayer_private *priv = (dblayer_private *)li->li_dblayer_private;
 
     return priv->dblayer_rm_db_file_fn(be, a, use_lock, no_force_chkpt);
@@ -1414,6 +1421,14 @@
     return;
 }
 
+void
+dblayer_destroy_txn_stack(void)
+{
+    /* Cleanup for the main thread to avoid false/positive leaks from libasan */
+    void *txn_stack = PR_GetThreadPrivate(thread_private_txn_stack);
+    PR_SetThreadPrivate(thread_private_txn_stack, NULL);
+}
+
 const char *
 dblayer_get_db_suffix(Slapi_Backend *be)
 {

ldap/servers/slapd/back-ldbm/ldbm_attr.c

@@ -54,7 +54,7 @@ attrinfo_delete(struct attrinfo **pp)
         idl_release_private(*pp);
         (*pp)->ai_key_cmp_fn = NULL;
         slapi_ch_free((void **)&((*pp)->ai_type));
-        slapi_ch_free((void **)(*pp)->ai_index_rules);
+        charray_free((*pp)->ai_index_rules);
         slapi_ch_free((void **)&((*pp)->ai_attrcrypt));
         attr_done(&((*pp)->ai_sattr));
         attrinfo_delete_idlistinfo(&(*pp)->ai_idlistinfo);

ldap/servers/slapd/back-ldbm/matchrule.c

@@ -107,7 +107,7 @@ destroy_matchrule_indexer(Slapi_PBlock *pb)
  * is destroyed
  */
 int
-matchrule_values_to_keys(Slapi_PBlock *pb, struct berval **input_values, struct berval ***output_values)
+matchrule_values_to_keys(Slapi_PBlock *pb, Slapi_Value **input_values, struct berval ***output_values)
 {
     IFP mrINDEX = NULL;
 
@@ -135,10 +135,8 @@ matchrule_values_to_keys_sv(Slapi_PBlock *pb, Slapi_Value **input_values, Slapi_
     slapi_pblock_get(pb, SLAPI_PLUGIN_MR_INDEX_SV_FN, &mrINDEX);
     if (NULL == mrINDEX) { /* old school - does not have SV function */
         int rc;
-        struct berval **bvi = NULL, **bvo = NULL;
-        valuearray_get_bervalarray(input_values, &bvi);
-        rc = matchrule_values_to_keys(pb, bvi, &bvo);
-        ber_bvecfree(bvi);
+        struct berval **bvo = NULL;
+        rc = matchrule_values_to_keys(pb, input_values, &bvo);
         /* note - the indexer owns bvo and will free it when destroyed */
         valuearray_init_bervalarray(bvo, output_values);
         /* store output values in SV form - caller expects SLAPI_PLUGIN_MR_KEYS is Slapi_Value** */

ldap/servers/slapd/back-ldbm/proto-back-ldbm.h

@@ -88,6 +88,7 @@ int dblayer_erase_index_file(backend *be, struct attrinfo *a, PRBool use_lock, i
 int dblayer_get_id2entry(backend *be, dbi_db_t **ppDB);
 int dblayer_get_changelog(backend *be, dbi_db_t ** ppDB, int create);
 int dblayer_release_id2entry(backend *be, dbi_db_t *pDB);
+void dblayer_destroy_txn_stack(void);
 int dblayer_txn_init(struct ldbminfo *li, back_txn *txn);
 int dblayer_txn_begin(backend *be, back_txnid parent_txn, back_txn *txn);
 int dblayer_txn_begin_ext(struct ldbminfo *li, back_txnid parent_txn, back_txn *txn, PRBool use_lock);
@@ -541,7 +542,7 @@ int compute_allids_limit(Slapi_PBlock *pb, struct ldbminfo *li);
  */
 int create_matchrule_indexer(Slapi_PBlock **pb, char *matchrule, char *type);
 int destroy_matchrule_indexer(Slapi_PBlock *pb);
-int matchrule_values_to_keys(Slapi_PBlock *pb, struct berval **input_values, struct berval ***output_values);
+int matchrule_values_to_keys(Slapi_PBlock *pb, Slapi_Value **input_values, struct berval ***output_values);
 int matchrule_values_to_keys_sv(Slapi_PBlock *pb, Slapi_Value **input_values, Slapi_Value ***output_values);
 
 /*