Fix reference processing crash (#309)

Fixed a bug that when enqueuing references, it swapped the reference
pending list with the last node of the new linked list. It should swap
with the *first* node instead.

Worked around a fact that the list given by the method
`ReferenceGlue::enqueue_references` may contain duplicated elements.
This is because the reference processor in mmtk-core was designed to
work with both of the two use patterns: (1) registering references when
created, and (2) discovering references while tracing. Therefore, the
reference processor kept the list of Reference instances discovered in
the previous GC (in the from-space). But as the OpenJDK binding scans
new Reference instances, it will add to-space addresses of the same
Reference instances to the reference processor. After the references are
processed, both pointers will be normalized to the two-space copy, but
as two different entries. We work around this problem in the OpenJDK
binding by simply skipping visited entries.

Author: wks

.github/scripts/ci-test-only-weak-ref.sh

@@ -1,50 +1,5 @@
 set -xe
 
-. $(dirname "$0")/common.sh
-
-unset JAVA_TOOL_OPTIONS
-cd $OPENJDK_PATH
-
+# Just run everything in ci-test-only-normal.sh, but with reference processing enabled.
 export MMTK_NO_REFERENCE_TYPES=false
-# Just test Immix and MarkCompact
-# Immix - normal weak ref impl
-# MarkCompact - with extra ref forwarding
-
-run_all() {
-    heap_multiplier=$1
-
-    runbms_dacapo2006_with_heap_multiplier antlr $heap_multiplier
-    runbms_dacapo2006_with_heap_multiplier fop $heap_multiplier
-    runbms_dacapo2006_with_heap_multiplier luindex $heap_multiplier
-    runbms_dacapo2006_with_heap_multiplier pmd $heap_multiplier
-    runbms_dacapo2006_with_heap_multiplier hsqldb $heap_multiplier
-    # The test may fail. Skip it for now.
-    #/home/runner/work/mmtk-openjdk/mmtk-openjdk/bundles/jdk/bin/java -XX:+UseThirdPartyHeap -server -XX:MetaspaceSize=100M -Xms92M -Xmx92M -jar /home/runner/work/mmtk-openjdk/mmtk-openjdk/dacapo/dacapo-2006-10-MR2.jar eclipse
-    #[2024-01-15T04:42:55Z INFO  mmtk::memory_manager] Initialized MMTk with Immix (FixedHeapSize(96468992))
-    #===== DaCapo eclipse starting =====
-    #[2024-01-15T04:42:58Z INFO  mmtk::util::heap::gc_trigger] [POLL] immix: Triggering collection (23560/23552 pages)
-    #[2024-01-15T04:42:58Z INFO  mmtk::scheduler::gc_work] End of GC (5015/23552 pages, took 76 ms)
-    #<setting up workspace...>
-    #<creating projects..............................................................>
-    #
-    # A fatal error has been detected by the Java Runtime Environment:
-    #
-    #  SIGSEGV (0xb) at pc=0x00007f7dd4627dff, pid=2923, tid=2924
-    #
-    # JRE version: OpenJDK Runtime Environment (11.0.19) (fastdebug build 11.0.19-internal+0-adhoc.runner.openjdk)
-    # Java VM: OpenJDK 64-Bit Server VM (fastdebug 11.0.19-internal+0-adhoc.runner.openjdk, mixed mode, tiered, compressed oops, third-party gc, linux-amd64)
-    # Problematic frame:
-    # j  java.lang.invoke.LambdaFormEditor.getInCache(Ljava/lang/invoke/LambdaFormEditor$Transform;)Ljava/lang/invoke/LambdaForm;+175 [email protected]
-    #
-    # runbms_dacapo2006_with_heap_multiplier eclipse $heap_multiplier
-}
-
-# --- Immix ---
-export MMTK_PLAN=Immix
-
-run_all 4
-
-# --- MarkCompact ---
-export MMTK_PLAN=MarkCompact
-
-run_all 4
+. $(dirname "$0")/ci-test-only-normal.sh

openjdk/mmtkUpcalls.cpp

@@ -304,17 +304,26 @@ static void mmtk_enqueue_references(void** objects, size_t len) {
 
   MutexLocker x(Heap_lock);
 
-  oop prev = NULL;
-  for (size_t i = 0; i < len; i++) {
+  oop first = (oop) objects[0]; // This points to the first node of the linked list.
+  oop last = first; // This points to the last node of the linked list.
+
+  for (size_t i = 1; i < len; i++) {
     oop reff = (oop) objects[i];
-    if (prev != NULL) {
-      HeapAccess<AS_NO_KEEPALIVE>::oop_store_at(prev, java_lang_ref_Reference::discovered_offset, reff);
+    oop old_discovered = HeapAccess<AS_NO_KEEPALIVE>::oop_load_at(reff, java_lang_ref_Reference::discovered_offset);
+    if (old_discovered != NULL || old_discovered == last) {
+      // Note that `objects` may contain duplicated elements.
+      // Because we gradually discover references during tracing,
+      // the ReferenceProcessor in mmtk-core may contain both from-space and to-space references of the same object.
+      // After processing references, they will be forwarded and become to-space references.
+      // We skip references that already have the `discovered` field set because they have already been visited.
+      continue;
     }
-    prev = reff;
+    HeapAccess<AS_NO_KEEPALIVE>::oop_store_at(last, java_lang_ref_Reference::discovered_offset, reff);
+    last = reff;
   }
 
-  oop old = Universe::swap_reference_pending_list(prev);
-  HeapAccess<AS_NO_KEEPALIVE>::oop_store_at(prev, java_lang_ref_Reference::discovered_offset, old);
+  oop old_first = Universe::swap_reference_pending_list(first);
+  HeapAccess<AS_NO_KEEPALIVE>::oop_store_at(last, java_lang_ref_Reference::discovered_offset, old_first);
   assert(Universe::has_reference_pending_list(), "Reference pending list is empty after swap");
 }