#15 fix race condition with xref

[balupillai]

Note

Medium Risk
Touches concurrency-sensitive GC xref bookkeeping; a small logic change, but mistakes could lead to missed external references or leaks under parallel tracing.

Overview
Tightens set_xref() so its forced CAS to notxref only happens when the object’s xref is unknown and was not already marked as an xref in the previous global-trace epoch.

This adds was_prev_epoch_xref() to detect prior-epoch xref values and prevents clobbering them, reducing a race where one thread could clear another thread’s xref mark across epoch flips.

^{Written by Cursor Bugbot for commit f2b518f. This will update automatically on new commits. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

[Copilot]

Pull request overview

This PR adjusts the GC “xref” bookkeeping during global tracing to reduce a race where forced normalization to notxref can clobber xref marks across epoch flips.

Changes:

• Add was_prev_epoch_xref() helper to detect whether an object’s current xref value corresponds to the previous global-trace epoch’s xref state.
• Tighten set_xref(..., force=1) so the CAS-to-notxref path only runs when the xref is unknown and not a previous-epoch xref.

Comments suppressed due to low confidence (1)

src/lgc.c:255

• The new !was_prev_epoch_xref(L, old_val) guard prevents set_xref(..., force=1) from ever converting a previous-epoch xref value to the current epoch’s definitive notxref. Since is_unknown_xref_val() treats previous-epoch values as “unknown”, leaving them unchanged means an object that was an xref last epoch can remain in an unknown state after the global trace; on the next epoch flip that stale value becomes the current definitive isxref, causing the object to be treated as externally referenced even if no xrefs remain (potentially preventing reclamation indefinitely). To avoid retaining stale xrefs, ensure previous-epoch xref values are still normalized to the current epoch’s definitive state by the end of a global trace (e.g., don’t skip the CAS here, or add an alternative path that clears/recomputes them once all heaps have been scanned).

    if (is_unknown_xref_val(L, old_val) && !was_prev_epoch_xref(L, old_val)) {
      /* Here's the issue: There may be another thread marking this as an xref,
       * and if we mark it as _not_ an xref at the same time, we're gonna have
       * a bad time.  So, we:
       *
       * 1. Load the value and save it locally.
       * 2. Check if it's unknown.
       * 3. If it's unknown and not xref previously, we run atomic CAS to compare it against the old
       *    value and make it 'not' xref.  If somebody else changed it (either to
       *    not an xref, or an xref), that's cool.
       */
      ck_pr_cas_32(&rval->xref, old_val, ck_pr_load_32(&G(L)->notxref));
    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.