Skip to content

Add regression tests for task stack scanning by the GC - #1422

Open
fingolfin wants to merge 2 commits into
masterfrom
gc-task-tests
Open

Add regression tests for task stack scanning by the GC#1422
fingolfin wants to merge 2 commits into
masterfrom
gc-task-tests

Conversation

@fingolfin

Copy link
Copy Markdown
Member

A GAP object held only in a C stack frame of a Julia task must survive collections that run while the task is suspended (parked in a Julia call, or busy on another thread). This went wrong when the task stack rescan optimization in GAP's julia_gc.c skipped old tasks, the cause of long-elusive crashes; see #1032, #1224 and gap-system/gap#6525.

The tests hold a canary object solely via a wrapper bag in a GAP kernel argument temporary and detect premature collection through per-canary finalizers, covering automatic and explicit incremental collections and, in a -t2 subprocess, a never-switching task on a second thread. Only automatic collections exercise the historically buggy path, so the driver churns allocations rather than calling GC.gc.

Resolves #996

AI disclosure: tests developed with the assistance of Claude Code.

A GAP object held only in a C stack frame of a Julia task must survive
collections that run while the task is suspended (parked in a Julia
call, or busy on another thread). This went wrong when the task stack
rescan optimization in GAP's julia_gc.c skipped old tasks, the cause of
long-elusive crashes; see #1032, #1224 and gap-system/gap#6525.

The tests hold a canary object solely via a wrapper bag in a GAP kernel
argument temporary and detect premature collection through per-canary
finalizers, covering automatic and explicit incremental collections
and, in a subprocess with two Julia threads and two GC mark threads, a
never-switching task on a second thread. Only automatic collections
exercise the historically buggy path, so the driver churns allocations
rather than calling GC.gc.

Fixes #996.

AI disclosure: tests developed with the assistance of Claude Code.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.35%. Comparing base (d9ac62d) to head (656aef4).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1422      +/-   ##
==========================================
+ Coverage   83.33%   83.35%   +0.01%     
==========================================
  Files          62       62              
  Lines        5048     5048              
  Branches       27       27              
==========================================
+ Hits         4207     4208       +1     
+ Misses        841      840       -1     

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Since the patch loop moved into the "Build GAP" step (b115fdc), the
relative patchdir resolved inside the GAP checkout in /tmp/GAPROOT, so
the patches -- including the julia_gc-rescan.patch working around the
task stack scanning bug -- were silently never applied. The new GC
task tests detect exactly that bug, turning the silent gap into CI
failures.

AI disclosure: diagnosed and fixed with the assistance of Claude Code.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@lgoettgens
lgoettgens self-requested a review August 31, 2026 10:19
Comment thread .github/workflows/gap.yml
mv GAPROOT /tmp/GAPROOT
cd /tmp/GAPROOT
patchdir=.github/workflows/GAP_patches/${{ matrix.gap-version }}
patchdir=${GITHUB_WORKSPACE}/.github/workflows/GAP_patches/${{ matrix.gap-version }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit unrelated to the rest of this PR (but looks fine anyway)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add tests involving Julia tasks

2 participants