Skip to content

The census and build_subsets.py never close the record files they read (not #118) #135

Description

@realmarcin

Found in the adversarial review of #120.

The skill and the PR blame the census ResourceWarning flood on #118, but #118 tracks unclosed write handles, not the per-record read leak

Where: .claude/skills/update-xmech-page/SKILL.md:128

The skill says 'The census opens every record without closing it (#118)', and the PR body says 'the census never closes its files (#118)'. #118 is about json.dump(doc, open(...)) write handles in build_subsets.py and the census, plus atomic writes. The census write was already fixed with a with block. The flood comes from the per-record reads, txt=open(f,...).read() at prefix_census.py:29 and build_subsets.py:98, and no issue tracks those reads.

Failure scenario. Someone fixes #118 as written (write handles and atomic rename) and closes it. The read leak that makes -W error::ResourceWarning unusable stays in place with no open issue, and the skill's warning now cites a closed issue that did not cover it.

Evidence. gh issue view 118: 'The remaining writes in build_subsets.py … and the census still use json.dump(doc, open(...)) without closing the handle … write to a temporary file and rename'. It does not mention record reads. scripts/fleet/prefix_census.py:29 has try: txt=open(f,encoding="utf-8",errors="ignore").read(). prefix_census.py:53 already uses with open(...) for the write.

Verifier (confirmed, low). The skill and the PR body are right that the census leaves every record it reads open, and right about what that does under -W error::ResourceWarning. The issue number they cite is wrong. #118 covers write handles and atomic rename in build_subsets.py and build_data.py. Its census wording was already stale: the census write has used a with block since #102, before this PR. #118 says nothing about the open(f).read() per-record reads at prefix_census.py:29 and build_subsets.py:98, and no other issue tracks them. The reviewer's failure scenario holds. Someone could fix #118 as written and close it, and the read leak that floods the log would stay with no open issue, while the skill kept pointing at the closed issue as its explanation. Severity stays low. The advice itself ("do not run with -W error::ResourceWarning") is still correct and nothing breaks. The damage is a wrong issue reference in procedure docs, which could let the real leak go untracked.

Suggested fix. File an issue for the unclosed per-record reads and cite it here instead of #118, or add a comment on #118 widening its scope.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions