Skip to content

test: give GraalPy's finaliser time to run after gc.collect (#1963) - #2010

Merged
JarryShaw merged 1 commit into
mainfrom
test/1963-graalpy-main-run
Oct 21, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
test/1963-graalpy-main-run

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • test — tests only

Description of your pull request and other information

On GraalPy, test_finaliser_alone_releases_an_abandoned_handle failed on main's first full GraalPy run (job 118912680486).

GraalPy's gc.collect forces a Java GC, sleeps briefly, and then runs only the weak-reference callbacks that are queued by that point (GcModuleBuiltins.javaCollect, graal-25.4.4). Under load, a weakref.finalize can therefore run after collect returns.

A new collect_until helper keeps collecting for up to FINALISER_GRACE: 10 s on GraalPy, 0 on CPython. A finaliser that never runs still fails. A stand-in finaliser that runs only on the third collection shows the helper waits for it. I could not reproduce the failure locally, so the cause is inferred from GraalPy's source.

The run's nine other FAILED ids were worker crashes, not test failures. Each worker hit --vm.Xmx2g with ExitOnOutOfMemoryError after its heap grew across tests. Each of those tests passes alone on GraalPy, so gating them would be wrong. The fix for the crashes goes to the GraalPy leg separately.

Overlaps #2000 in test_extraction_ownership.py; I'll rebase whichever lands second.

Part of #1963.

@JarryShaw JarryShaw added test Pull requests that add or correct tests (test: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 21, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 21, 2026
@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 21, 2026
…1963)

- GraalPy's gc.collect forces a full Java GC, sleeps 15 ms and runs only
  the weak reference callbacks queued by then (GcModuleBuiltins.javaCollect,
  graal-25.4.4), so on a loaded runner the #1997 finaliser can run after it
  returns; job 118912680486 failed test_finaliser_alone_releases_an_abandoned_handle
  with the handle still open
- new collect_until keeps collecting up to FINALISER_GRACE, 10 s on GraalPy
  and none on CPython, so a finaliser that never runs still fails; used there
  and in test_extractor_abandoned_before_eof_still_releases_its_handle
- new stand-in test pins that the helper waits out a late finaliser

The file passes on CPython and GraalPy 25.4.4; the tier guard passes.

Refs #1963
@JarryShaw
JarryShaw force-pushed the test/1963-graalpy-main-run branch from 190b489 to c33e6b7 Compare October 21, 2026 00:22
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at c33e6b797. A Sonnet review of 190b48930 checked GraalPy's GcModuleBuiltins.javaCollect at graal-25.4.4: it calls forceFullGC(), sleeps 15 ms, then polls only the reference queue as it stands. That is the race this PR describes.

  • FINALISER_GRACE keys on sys.implementation.name, and at 0 on CPython a late finaliser still fails.
  • The loop is bounded by time.monotonic().
  • The third-collection stand-in shows the helper waits.

c33e6b797 applies the same helper to test_extractor_abandoned_before_eof_still_releases_its_handle, the review's one gap; the diff is that one line, and I checked it myself. The file passes on CPython (16) and on GraalPy 25.4.4 with the leg's heap flags (16).

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.54% (unit tier, Python 3.15, c33e6b797, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19597 881 2618 867 92.11%
pcapkit/corekit 2540 71 808 37 96.54%
pcapkit/dumpkit 276 3 100 3 98.40%
pcapkit/foundation 2929 23 1020 25 98.78%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17468 222 4600 187 98.13%
pcapkit/toolkit 872 36 298 10 95.21%
pcapkit/utilities 523 2 158 4 99.12%
pcapkit/vendor 4431 1995 1018 167 51.73%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit a95953e into main Oct 21, 2026
57 checks passed
@JarryShaw
JarryShaw deleted the test/1963-graalpy-main-run branch October 21, 2026 04:09
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 21, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 21, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Pull requests that add or correct tests (test: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant