fix race condition in MapMerge duplicate handling under fork-join (#2595)

Author: GuntherRademacher

basex-core/src/main/java/org/basex/query/func/map/MapBuild.java

@@ -33,7 +33,7 @@ public XQMap item(final QueryContext qc, final InputInfo ii) throws QueryExcepti
       final Iter iter = (keys != null ? invoke(keys, args, qc) : item).atomIter(qc, info);
       for(Item key; (key = qc.next(iter)) != null;) {
         final Value old = builder.get(key);
-        final Value val = dups.merge(key, old, value != null ? invoke(value, args, qc) : item);
+        final Value val = dups.merge(key, old, value != null ? invoke(value, args, qc) : item, qc);
         if(val != null) builder.put(key, val);
       }
     }

basex-core/src/main/java/org/basex/query/func/map/MapDuplicates.java

@@ -17,22 +17,25 @@ public abstract class MapDuplicates {
    * @param key key
    * @param old old (can be {@code null})
    * @param value value
+   * @param qc query context
    * @return merged value, or new value if the old one is {@code null}
    * @throws QueryException query exception
    */
-  Value merge(final Item key, final Value old, final Value value) throws QueryException {
-    return old != null ? get(key, old, value) : value;
+  Value merge(final Item key, final Value old, final Value value, final QueryContext qc)
+      throws QueryException {
+    return old != null ? get(key, old, value, qc) : value;
   }
 
   /**
    * Merges the old and new value.
    * @param key key
    * @param old old
    * @param value value
+   * @param qc query context
    * @return merged value, or {@code null} if insertion of map entry can be skipped
    * @throws QueryException query exception
    */
-  abstract Value get(Item key, Value old, Value value) throws QueryException;
+  abstract Value get(Item key, Value old, Value value, QueryContext qc) throws QueryException;
 
   /**
    * Returns the result type.

basex-core/src/main/java/org/basex/query/func/map/MapMerge.java

@@ -69,7 +69,7 @@ public XQMap item(final QueryContext qc, final InputInfo ii) throws QueryExcepti
       final XQMap map = toMap(current);
       for(final Item key : map.keys()) {
         final Value old = mb != null ? mb.get(key) : mp.getOrNull(key);
-        final Value val = dups.merge(key, old, map.get(key));
+        final Value val = dups.merge(key, old, map.get(key), qc);
         if(val != null) {
           if(mb != null) mb.put(key, val);
           else mp = mp.put(key, val);
@@ -143,7 +143,7 @@ final MapDuplicates duplicates(final MergeOptions options, final QueryContext qc
     if(md != null) return md;
     // function
     final Value duplicates = options.get(MergeOptions.DUPLICATES);
-    if(duplicates instanceof FItem) return new Invoke(toFunction(duplicates, 2, qc), qc);
+    if(duplicates instanceof FItem) return new Invoke(toFunction(duplicates, 2, qc));
     // fixed option
     final String string = duplicates.isEmpty() ? dflt.toString() : toString(duplicates, qc);
     final Duplicates value = Enums.get(Duplicates.class, string);
@@ -152,7 +152,7 @@ final MapDuplicates duplicates(final MergeOptions options, final QueryContext qc
 
     return switch(value) {
       case REJECT -> new Reject();
-      case COMBINE -> new Combine(qc);
+      case COMBINE -> new Combine();
       case USE_FIRST -> new UseFirst();
       default -> new UseLast();
     };
@@ -178,7 +178,7 @@ public int hofOffsets() {
    */
   static final class UseFirst extends MapDuplicates {
     @Override
-    Value get(final Item key, final Value old, final Value value) {
+    Value get(final Item key, final Value old, final Value value, final QueryContext qc) {
       return null;
     }
   }
@@ -188,7 +188,7 @@ Value get(final Item key, final Value old, final Value value) {
    */
   static final class UseLast extends MapDuplicates {
     @Override
-    Value get(final Item key, final Value old, final Value value) {
+    Value get(final Item key, final Value old, final Value value, final QueryContext qc) {
       return value;
     }
   }
@@ -197,19 +197,8 @@ Value get(final Item key, final Value old, final Value value) {
    * Concatenate values.
    */
   static final class Combine extends MapDuplicates {
-    /** Query context. */
-    private final QueryContext qc;
-
-    /**
-     * Constructor.
-     * @param qc query context
-     */
-    Combine(final QueryContext qc) {
-      this.qc = qc;
-    }
-
     @Override
-    Value get(final Item key, final Value old, final Value value) {
+    Value get(final Item key, final Value old, final Value value, final QueryContext qc) {
       return old.append(value, qc);
     }
 
@@ -224,7 +213,8 @@ SeqType type(final SeqType st) {
    */
   final class Reject extends MapDuplicates {
     @Override
-    Value get(final Item key, final Value old, final Value value) throws QueryException {
+    Value get(final Item key, final Value old, final Value value, final QueryContext qc)
+        throws QueryException {
       throw MERGE_DUPLICATE_X.get(info, key);
     }
   }
@@ -235,25 +225,22 @@ Value get(final Item key, final Value old, final Value value) throws QueryExcept
   final class Invoke extends MapDuplicates {
     /** Combiner function. */
     private final FItem function;
-    /** Query context. */
-    private final QueryContext qc;
     /** HOF arguments. */
-    private final HofArgs args;
+    private final ThreadLocal<HofArgs> args;
 
     /**
      * Constructor.
      * @param function combiner function
-     * @param qc query context
      */
-    Invoke(final FItem function, final QueryContext qc) {
+    Invoke(final FItem function) {
       this.function = function;
-      this.qc = qc;
-      args = new HofArgs(2);
+      args = ThreadLocal.withInitial(() -> new HofArgs(2));
     }
 
     @Override
-    Value get(final Item key, final Value old, final Value value) throws QueryException {
-      return function.invoke(qc, info, args.set(0, old).set(1, value).get());
+    Value get(final Item key, final Value old, final Value value, final QueryContext qc)
+        throws QueryException {
+      return function.invoke(qc, info, args.get().set(0, old).set(1, value).get());
     }
 
     @Override

basex-core/src/test/java/org/basex/query/func/XQueryModuleTest.java

@@ -152,6 +152,26 @@ public final class XQueryModuleTest extends SandboxTest {
     query(func.args(" (true#0, false#0)", " { 'parallel': 1000000000 }"), "true\nfalse");
     query(func.args(" (true#0, false#0)", " { 'parallel': <_>1</_> }"), "true\nfalse");
 
+    // MapMerge mutable instance variables
+    query("xquery:fork-join(\r\n"
+        + "  for $t in 1 to 100\r\n"
+        + "  return function() {\r\n"
+        + "    for $i in 1 to 10\r\n"
+        + "    let $v := map:build(\r\n"
+        + "      ($i, $i),\r\n"
+        + "      (),\r\n"
+        + "      (),\r\n"
+        + "      { 'duplicates': function($x, $y) { $x * $y } }\r\n"
+        + "    )($i)\r\n"
+        + "    return\r\n"
+        + "      if($v eq $i * $i) then\r\n"
+        + "        $v\r\n"
+        + "      else\r\n"
+        + "        error(xs:QName('err:FAIL'), `{$i}: expected {$i * $i}, but got {$v}`)\r\n"
+        + "  }\r\n"
+        + ")\r\n"
+        + "=> count()", 1000);
+
     // optimizations
     check(func.args(" ()"), "", empty());
     check(func.args(" false#0"), false, root(DynFuncCall.class));