Fail safe the canary chain walk and retire dead marker-tag plumbing

buildCanaryChainEvent()'s parent_tag walk now bounds at the frontier's
maxCapacity() like reconstructChain() and returns false on a cyclic or
corrupt chain instead of spinning the poll thread; the walk's
event._root_kind now comes from the terminal root-attached entry rather
than the parent-side interior entry (always 0), matching the
appendStaticFieldRootType(terminal) call and suppressChainEvent()'s
transient-root gate.

Remove the retired canary marker-tag mechanism end to end: nothing
pre-tags candidates anymore, so the decode branches in
heapReferenceCallback() and pollWatchedTargets() were unreachable - and
runPass()'s marker-release loop was worse than dead: with every
_candidate_tags[i] left at 0 it asked GetObjectsWithTags to enumerate
ALL untagged objects once per candidate slot on every search stop.

Also: read the container usage file from the hierarchy that supplied
the winning limit instead of probing v2/v1 filenames in order; clamp
PainBudget::drain() against a backward clock step; refuse symlinked or
non-owned rc-debug override files under /tmp; resolve each container
interface tag independently instead of failing the whole call; reset
consume_tier_fair()'s cursor on a completed-but-unproductive lap; drop
the pod fixture's dead placeholder assignment and assert the
representative carries its leak tag. Tests cover the bounded walk, the
deep-chain reconstruction, the symlink rejection and the cycle
fail-safe.

Author: jbachorik

ddprof-lib/src/main/cpp/os_linux.cpp

@@ -918,6 +918,14 @@ int OS::getCgroupCpuMillicores() {
 // delay the OOM projection.
 static char g_memory_limit_cgroup_path[PATH_MAX] = {0};
 
+// Which cgroup hierarchy (v2 or v1) supplied the winning limit recorded in
+// g_memory_limit_cgroup_path: the usage file must be read from the SAME
+// hierarchy (v2 memory.current vs v1 memory.usage_in_bytes), not probed by
+// filename order - on a hybrid system both controller files can be visible
+// for the same cgroup dir, and reading the wrong one pairs the limit with an
+// unrelated usage number.
+static bool g_memory_limit_cgroup_v2 = true;
+
 static long walkCgroupV2MemoryLimit(char* path, char* winner_path_out) {
     size_t base_len = strlen("/sys/fs/cgroup");
     long best = -1;
@@ -1002,6 +1010,7 @@ long OS::getContainerMemoryLimit() {
                 int fd = open(leaf, O_RDONLY);
                 if (fd != -1) {
                     close(fd);
+                    g_memory_limit_cgroup_v2 = true;
                     return walkCgroupV2MemoryLimit(path, g_memory_limit_cgroup_path);
                 }
             }
@@ -1022,6 +1031,7 @@ long OS::getContainerMemoryLimit() {
                 int fd = open(leaf, O_RDONLY);
                 if (fd != -1) {
                     close(fd);
+                    g_memory_limit_cgroup_v2 = false;
                     return walkCgroupV1MemoryLimit(path, g_memory_limit_cgroup_path);
                 }
             }
@@ -1045,26 +1055,24 @@ long OS::getContainerMemoryUsage() {
 
     // Same cgroup the winning limit came from, if the limit walk recorded
     // one - read its usage first, falling back to the process's own leaf.
-    // v2 names the file memory.current, v1 memory.usage_in_bytes - try both.
+    // The usage filename follows the hierarchy that supplied the limit (the
+    // recorded winner's hierarchy is authoritative, not filename order).
     if (g_memory_limit_cgroup_path[0] != '\0') {
         char file[PATH_MAX];
-        const char *usage_files[] = {"%s/memory.current", "%s/memory.usage_in_bytes"};
-        for (const char *fmt : usage_files) {
-            if ((size_t)snprintf(file, sizeof(file), fmt,
-                                 g_memory_limit_cgroup_path) >= sizeof(file)) {
-                continue;
-            }
+        const char *fmt = g_memory_limit_cgroup_v2 ? "%s/memory.current"
+                                                   : "%s/memory.usage_in_bytes";
+        if ((size_t)snprintf(file, sizeof(file), fmt,
+                             g_memory_limit_cgroup_path) < sizeof(file)) {
             int fd = open(file, O_RDONLY);
-            if (fd == -1) {
-                continue;
-            }
-            char buf[32] = {0};
-            ssize_t r = read(fd, buf, sizeof(buf) - 1);
-            close(fd);
-            if (r > 0) {
-                long usage = atol(buf);
-                if (usage >= 0) {
-                    return usage;
+            if (fd != -1) {
+                char buf[32] = {0};
+                ssize_t r = read(fd, buf, sizeof(buf) - 1);
+                close(fd);
+                if (r > 0) {
+                    long usage = atol(buf);
+                    if (usage >= 0) {
+                        return usage;
+                    }
                 }
             }
         }

ddprof-lib/src/main/cpp/painBudget.h

@@ -41,6 +41,15 @@ class PainBudget {
       _last_update_ns = now_ns;
       return;
     }
+    if (now_ns <= _last_update_ns) {
+      // Clock stepped backward (or a test passed a literal smaller than the
+      // OS::nanotime()-seeded baseline): the unsigned subtraction would wrap
+      // to ~2^64 ns and drain the entire debt in one call. The failure
+      // direction is permissive (a blocked restart becomes allowed), so clamp
+      // to zero elapsed and re-baseline instead.
+      _last_update_ns = now_ns;
+      return;
+    }
     u64 elapsed_ns = now_ns - _last_update_ns;
     double elapsed_ms = (double)elapsed_ns / 1000000.0;
     // _refill_rate == 0.0 (the default constructor argument) makes this a

ddprof-lib/src/main/cpp/referenceChainAnchors.cpp

@@ -254,6 +254,24 @@ ReferenceChainTracker::collectStaticFieldAnchorsForRotation(int max_count) {
     }
     if (took > 0) {
       cursor = consumed_pos + 1 >= idx_size ? 0 : consumed_pos + 1;
+    } else {
+      // Took nothing AND no pick sits at or ahead of the cursor: this lap
+      // already passed every current member of the tier (members selected in
+      // earlier calls and since demoted out of eligibility). Without a reset
+      // the cursor never wraps again - every later call skips them all
+      // (p.pos < cursor) and the tier starves until a NEW anchor is appended
+      // at a higher pos. Treat the lap as completed-but-unproductive and
+      // restart it, matching the wrap semantics applied above.
+      bool any_ahead = false;
+      for (const TierPick &p : picks) {
+        if (p.pos >= cursor) {
+          any_ahead = true;
+          break;
+        }
+      }
+      if (!any_ahead) {
+        cursor = 0;
+      }
     }
     return took;
   };
@@ -346,10 +364,19 @@ bool ReferenceChainTracker::resolveContainerInterfaceTags(
     if (*iface.tag_out != 0) {
       continue;
     }
+    // Each interface is resolved independently: a transient JVMTI error on
+    // one of them (GetTag/SetTag/FindClass failing for exactly one) must not
+    // abort the other's resolution - the already-resolved tag stays cached in
+    // its slot either way, and a per-interface failure only leaves THAT tag
+    // at 0 for this call (the caller treats a false return as "shapes
+    // unknown this pass" and retries on the next reconcile). Failing the
+    // whole call on the first error would keep both anchors unclassified for
+    // as long as one interface keeps erroring, even though the other resolved
+    // fine.
     jclass local = jni->FindClass(iface.name);
     if (jniExceptionCheck(jni) || local == nullptr) {
       jni->ExceptionClear();
-      return false;
+      continue;
     }
     jlong tag = 0;
     bool ok = jvmti->GetTag(local, &tag) == JVMTI_ERROR_NONE;
@@ -375,9 +402,9 @@ bool ReferenceChainTracker::resolveContainerInterfaceTags(
       *iface.tag_out = tag;
     }
     jni->DeleteLocalRef(local);
-    if (!ok) {
-      return false;
-    }
+    // A per-interface GetTag/SetTag failure leaves that tag at 0 - the final
+    // check below reports false only if an interface genuinely has no tag,
+    // never as an early abort that skips the other interface.
   }
   return _collection_iface_class_tag != 0 && _map_iface_class_tag != 0;
 }

ddprof-lib/src/main/cpp/referenceChainEvents.cpp

@@ -171,8 +171,27 @@ bool ReferenceChainTracker::buildCanaryChainEvent(int candidate_idx,
                        (long long)parent_tag, candidate_idx);
       return false;
     }
-    root_kind = entry.root_kind;
-    for (jlong tag = parent_tag; tag > 0;) {
+    // Bounded like every sibling walk over this same parent chain:
+    // FrontierTable::reconstructChain() bounds at maxCapacity() hops and
+    // returns false on a "cyclic or corrupt parent chain",
+    // requeueChainRootForRotation() bounds at _hop_cap, and improveChain()'s
+    // cycle guard bounds at 4096. improveChain() structurally prevents cycles
+    // (it refuses a parent whose chain routes through the entry), but the
+    // table's contents are also written by insert() with no such validation,
+    // so a corrupt chain must fail safe instead of spinning this poll-thread
+    // walk forever: every tag maps to a distinct slot (tags are never
+    // reused), so a well-formed chain can visit at most maxCapacity() entries
+    // before reaching parent_tag == 0 or repeating a slot.
+    const int hop_bound = _frontier->maxCapacity();
+    int hops = 0;
+    for (jlong tag = parent_tag; tag > 0; hops++) {
+      if (hops > hop_bound) {
+        TEST_LOG_SUMMARY("ReferenceChainTracker::buildCanaryChainEvent false: "
+                         "chain walk exceeded hop bound (%d) - cyclic or "
+                         "corrupt parent chain (candidate=%d)",
+                         hops, candidate_idx);
+        return false;
+      }
       if (!_frontier->lookup(tag, &entry)) {
         TEST_LOG_SUMMARY("ReferenceChainTracker::buildCanaryChainEvent false: "
                          "chain walk: tag=%lld not in frontier (candidate=%d)",
@@ -182,6 +201,14 @@ bool ReferenceChainTracker::buildCanaryChainEvent(int candidate_idx,
       chain.push_back(entry.referrer_klass);
       tag = entry.parent_tag;
     }
+    // The root kind describes the chain's ROOT, not the candidate-side parent:
+    // the walk's last iteration is always the root-attached entry (parent_tag
+    // == 0, root_kind != 0), so `entry` holds it here - same terminal-root
+    // semantics reconstructChain() uses for *out_root_kind. The parent-side
+    // entry read before the loop would almost always yield 0 (interior entries
+    // carry root_kind == 0), misreporting every walk-reconstructed chain and
+    // defeating suppressChainEvent()'s transient-root gate.
+    root_kind = entry.root_kind;
     terminal = entry;
   } else if (parent_tag == 0 && frontier_tag > 0) {
     // Root-referenced candidate: chain is just [candidate_klass].
@@ -305,14 +332,12 @@ void ReferenceChainTracker::pollWatchedTargets(jvmtiEnv *jvmti, JNIEnv *jni) {
                  klass_id, MAX_LEAK_CANDIDATES_FROM_LT);
         continue;
       }
-      // Tag this candidate's specific representative object with a distinct marker tag
-      // (MARKER_TAG_BASE - slot) so heapReferenceCallback() can identify that exact object by
-      // identity when the walk reaches it - matching by class alone would record a chain for
-      // whichever instance of that class the walk happens to visit, not necessarily the one
-      // LivenessTracker flagged as growing.
+      // Candidate admission: the marker->leak-tag migration retired
+      // pre-tagging the representative object (the retired marker-tag decode
+      // branches are gone); this candidate is discovered when the walk or the
+      // poll intercepts one of its leak-tagged instances.
       int slot = _candidate_count;
       _candidate_klass_ids[slot] = klass_id;
-      _candidate_tags[slot] = 0; // no marker tags — using leak tags now
       _candidate_count = slot + 1;
       TEST_LOG_SUMMARY("ReferenceChainTracker::pollWatchedTargets canary: admitted klass_id=%u "
                "into slot=%d (candidate_count now %d)",
@@ -428,57 +453,19 @@ void ReferenceChainTracker::pollWatchedTargets(jvmtiEnv *jvmti, JNIEnv *jni) {
     // Read the existing tag; seeding here would bypass the forward walk.
     jlong tag = getTag(jvmti, obj);
 
-    // Canary search: if this candidate was pre-tagged with a marker tag (negative, set above), use
-    // the canary chain reconstruction.
-    if (tag <= MARKER_TAG_BASE) {
-      // The marker tag encodes the slot this object was pre-tagged at (MARKER_TAG_BASE - slot,
-      // mirroring heapReferenceCallback()'s own decode at its MARKER_TAG_BASE check).
-      int candidate_slot = (int)(MARKER_TAG_BASE - tag);
-      if (candidate_slot < 0 || candidate_slot >= _candidate_count) {
-        TEST_LOG_SUMMARY("ReferenceChainTracker::pollWatchedTargets canary candidate[%d] "
-                 "klass_id=%u marker_tag=%lld decodes to out-of-range slot=%d "
-                 "(candidate_count=%d) - skipping",
-                 i, klass_id, (long long)tag, candidate_slot, _candidate_count);
-        jni->DeleteLocalRef(obj);
-        continue;
-      }
-      bool need_refresh = false;
-      jlong canary_ftag = _candidate_frontier_tags[candidate_slot];
-      _resolved_chains_lock.lock();
-      auto it = _resolved_chains.find(canary_ftag);
-      need_refresh = (it == _resolved_chains.end() ||
-                      it->second.source_search_ns != current_search_ns);
-      _resolved_chains_lock.unlock();
-      TEST_LOG_SUMMARY("ReferenceChainTracker::pollWatchedTargets canary candidate[%d] "
-               "klass_id=%u marker_tag=%lld slot=%d needRefresh=%d",
-               i, klass_id, (long long)tag, candidate_slot, need_refresh);
-      if (need_refresh) {
-        ReferenceChainEvent event;
-        bool built = buildCanaryChainEvent(candidate_slot, &event);
-        TEST_LOG_SUMMARY("ReferenceChainTracker::pollWatchedTargets canary "
-                 "buildCanaryChainEvent(slot=%d) -> %d",
-                 candidate_slot, built);
-        if (built && suppressChainEvent(event)) {
-          TEST_LOG("ReferenceChainTracker::pollWatchedTargets "
-                   "filtered depth=%u root_kind=%d canary_ftag=%lld "
-                   "klass_id=%u (canary path)",
-                   event._depth, (int)event._root_kind,
-                   (long long)canary_ftag, klass_id);
-          built = false;
-          invalidateResolvedChain(canary_ftag);
-        }
-        if (built) {
-          event._start_time = TSC::ticks();
-          cacheResolvedChain(canary_ftag, std::move(event),
-                              canary_ftag, current_search_ns);
-        }
-      }
-      // Fall through to discovered-instances check below — the canary representative may not have
-      // been reached by BFS yet, but other instances of the same class may have been admitted and
-      // their chains can be built now.
-    }
-
-    // Normal (non-canary) path: tag > 0 means the walk visited this object and assigned it a
+    // NOTE: the retired canary marker-tag decode used to live here (an object
+    // pre-tagged with MARKER_TAG_BASE - slot took the legacy canary chain
+    // reconstruction). The marker->leak-tag migration stopped pre-tagging
+    // candidate representatives entirely - no JVMTI tag in the process can
+    // ever be <= MARKER_TAG_BASE (-2^62): leak tags are positive
+    // (LEAK_TAG_BASE), frontier tags positive, class tags small negative
+    // magnitudes - so the branch was unreachable, and a stale negative tag
+    // (a class tag) could never silently take the legacy canary path.
+    // Candidate chain reconstruction now runs through the leak-tag discovery
+    // block below and the dead-representative canary path earlier in this
+    // loop.
+
+    // Normal path: tag > 0 means the walk visited this object and assigned it a
     // frontier tag.
 
     // Keep the holder chain's root warm in the rotation queue: a growing container's current

ddprof-lib/src/main/cpp/referenceChainTraversal.cpp

@@ -1164,25 +1164,15 @@ bool ReferenceChainTracker::runPass(jvmtiEnv *jvmti, JNIEnv *jni,
 
   if (load(_search_state) != SearchState::RUNNING) {
     _tags_released = releaseSearchTags(jvmti, jni);
-    // Release canary marker tags: use GetObjectsWithTags to find all live marker-tagged objects and
-    // clear them.
     if (_candidate_count > 0) {
-      for (int i = 0; i < _candidate_count; i++) {
-        jlong tag = _candidate_tags[i];
-        jint count = 0;
-        jobject *objects = nullptr;
-        jlong *result_tags = nullptr;
-        jvmtiError cerr = jvmti->GetObjectsWithTags(
-            1, &tag, &count, &objects, &result_tags);
-        if (cerr == JVMTI_ERROR_NONE && count > 0) {
-          for (jint j = 0; j < count; j++) {
-            jvmti->SetTag(objects[j], 0);
-            jni->DeleteLocalRef(objects[j]);
-          }
-          jvmti->Deallocate((unsigned char *)objects);
-          jvmti->Deallocate((unsigned char *)result_tags);
-        }
-      }
+      // No marker-tag release pass: the marker->leak-tag migration retired
+      // pre-tagged candidate representatives (nothing sets _candidate_tags
+      // anymore), so there are no per-candidate marker JVMTI tags to clear -
+      // releaseSearchTags() above owns every live tag this search minted.
+      // (The old GetObjectsWithTags(1, &_candidate_tags[i]) loop here was
+      // worse than dead: with every _candidate_tags[i] left at 0 it asked
+      // JVMTI to enumerate ALL UNTAGGED objects - potentially the whole
+      // heap - once per candidate slot on every search stop.)
       _candidate_count = 0;
       _candidate_found_bits = 0;
       memset(_candidate_discovered_count, 0, sizeof(_candidate_discovered_count));

ddprof-lib/src/main/cpp/referenceChainWalk.cpp

@@ -168,59 +168,15 @@ jint JNICALL ReferenceChainTracker::heapReferenceCallback(
     edge_referrer_class_tag = *referrer_tag_ptr;
   }
 
-  // Canary pruning: if this object is the pre-tagged representative of a leaked candidate (marker
-  // tag MARKER_TAG_BASE - i, negative), record its chain link but do NOT enqueue its children
-  // (treat as a leaf).
-  if (ctx->tracker->_candidate_count > 0 &&
-      *tag_ptr <= ReferenceChainTracker::MARKER_TAG_BASE) {
-    int candidate_idx = (int)(ReferenceChainTracker::MARKER_TAG_BASE - *tag_ptr);
-    if (candidate_idx >= 0 && candidate_idx < ctx->tracker->_candidate_count) {
-      jlong rtag = (referrer_tag_ptr != nullptr) ? *referrer_tag_ptr : 0;
-      u32 candidate_klass = ctx->tracker->classTags()->resolve(class_tag);
-      if (rtag > 0) {
-        FrontierEntry parent{};
-        if (ctx->frontier->lookup(rtag, &parent)) {
-          // Use the marker tag itself as the frontier table key - it is already a unique
-          // per-candidate value, so no nextTag() is needed.
-          jlong frontier_tag = *tag_ptr;
-          ctx->frontier->insert(frontier_tag, rtag,
-                                  parent.referrer_klass,
-                                  parent.depth + 1,
-                                  FrontierEntryState::FRONTIER,
-                                  parent.root_kind,
-                                  /*class_tag=*/0, edge_field_index,
-                                  (u8)reference_kind);
-          ctx->tracker->_candidate_parent_tags[candidate_idx] = rtag;
-          ctx->tracker->_candidate_frontier_tags[candidate_idx] = frontier_tag;
-          ctx->tracker->_candidate_referrer_klasses[candidate_idx] = candidate_klass;
-          ctx->tracker->_candidate_depths[candidate_idx] = parent.depth + 1;
-          ctx->tracker->_candidate_found_bits |= (1ULL << candidate_idx);
-          TEST_LOG("ReferenceChainTracker::heapReferenceCallback canary "
-                 "pruned candidate %d (klass_id=%u frontier_tag=%lld)",
-                 candidate_idx, candidate_klass, (long long)frontier_tag);
-        }
-      } else {
-        // Root-referenced candidate.
-        jlong frontier_tag = *tag_ptr;
-        ctx->frontier->insert(frontier_tag, 0,
-                                candidate_klass, 1,
-                                FrontierEntryState::FRONTIER,
-                                (u8)reference_kind,
-                                /*class_tag=*/0, edge_field_index,
-                                /*edge_kind=*/0, edge_referrer_class_tag);
-        ctx->tracker->_candidate_parent_tags[candidate_idx] = 0;
-        ctx->tracker->_candidate_frontier_tags[candidate_idx] = frontier_tag;
-        ctx->tracker->_candidate_referrer_klasses[candidate_idx] = candidate_klass;
-        ctx->tracker->_candidate_depths[candidate_idx] = 1;
-        ctx->tracker->_candidate_found_bits |= (1ULL << candidate_idx);
-        TEST_LOG("ReferenceChainTracker::heapReferenceCallback canary "
-                 "pruned root-referenced candidate %d (klass_id=%u)",
-                 candidate_idx, candidate_klass);
-      }
-      // Do NOT enqueue children for this object.
-      return 0;
-    }
-  }
+  // NOTE: the retired canary marker-tag decode used to live here (objects
+  // pre-tagged with MARKER_TAG_BASE - i were pruned as leaves and recorded as
+  // chain roots). The marker->leak-tag migration stopped pre-tagging candidate
+  // representatives entirely - no JVMTI tag in the process can ever be
+  // <= MARKER_TAG_BASE (-2^62): leak tags are positive (LEAK_TAG_BASE),
+  // frontier tags positive, class tags small negative magnitudes - so the
+  // branch was unreachable. Candidate discovery now runs exclusively through
+  // the leak-tag interception in pollWatchedTargets() (which also records
+  // _candidate_found_bits/_candidate_frontier_tags).
 
   if (*tag_ptr < 0) {
     if (ctx->static_field_seed &&