Extractor._cleanup closes the caller-supplied stream and leaks the file it opened itself

[JarryShaw]

Extractor._cleanup() closes the input file only when the caller supplied the stream, and never when Extractor opened it itself. Ownership is inverted: it leaks what it owns and closes what it does not.

Mechanism

pcapkit/foundation/extraction.py records how the input arrived:

self._flag_s = isinstance(fin, str)          # input filename flag
...
if self._flag_s:
    self._ifile = open(ifnm, 'rb')           # pcapkit opens it
else:
    ...                                       # caller supplied a stream

and _cleanup() at extraction.py:1095-1099 closes under the negation of that flag:

if isinstance(self._ifile, SeekableReader):
    self._ifile.close()
elif not self._flag_s:
    self._ifile.close()

So _flag_s is True — pcapkit opened the file — takes neither branch, because a plain open() returns a BufferedReader rather than a SeekableReader. The handle is never closed. _flag_s is False — the caller owns the stream — is closed, which is also the wrong choice on its own terms: closing a caller's file object is not Extractor's to do.

Measured

On origin/main (4529fdb1f), CPython 3.14.7, with ResourceWarning always enabled and an explicit gc.collect():

path given  (pcapkit opens it): 1 unclosed
stream given (caller owns it) : 0 unclosed

The leaked warning is the familiar one:

ResourceWarning: unclosed file <_io.BufferedReader name='.../examples/captures/in.pcap'>

Why this is worth fixing beyond tidiness

This is the production root cause of #606, the flake that reddened three unrelated pull requests — #577, #596 and #600 — and was observed migrating between two of them in eleven minutes with neither branch touched. #606's fix made the assertion in tests/utilities/test_stacklevel.py immune to foreign warnings, which is what made the CI signal safe. It did not stop the leak, and could not: the leak is here, in library code.

Confirmation that the leak survives #606: pytest tests/integration/test_runtime_extract.py tests/protocols/misc/pcap/test_frame_runtime.py -W always::ResourceWarning still emits two unclosed-in.pcap warnings. Any test or caller that passes a path leaks a descriptor, and on a long-running process that accumulates.

Extractor(fin=<path>) is the ordinary documented usage, so this affects normal callers rather than a corner.

Suggested direction

Close when _flag_s is true and leave the caller's stream alone when it is false — the inverse of the present condition. A context-manager protocol on Extractor would be the fuller answer, but the one-line inversion is the defect.

A regression test should assert on ResourceWarning for the path-given case specifically, since the stream-given case has always been quiet and would not catch a reintroduction.

Notes

• Found by the test_stacklevel asserts an exact warning count, so a leaked in.pcap handle fails it on unrelated PRs #606 worker, which correctly stopped rather than pushing it: test_stacklevel asserts an exact warning count, so a leaked in.pcap handle fails it on unrelated PRs #606 was authorised as a test-only direct push to main, and this is library code that needs a PR.
• Verified independently here by reading both sites and by the runtime comparison above.
• A correction to my own test_stacklevel asserts an exact warning count, so a leaked in.pcap handle fails it on unrelated PRs #606 issue while it is in view: the candidate list of leaking test files I put in test_stacklevel asserts an exact warning count, so a leaked in.pcap handle fails it on unrelated PRs #606 was both over- and under-inclusive. It missed tests/utilities/test_logging.py, which is the file that actually leaked into test_stacklevel's directory, because my crude close-ish tally was satisfied by an unrelated with open elsewhere in that file; and it wrongly included tests/_tiers.py and tests/test_tier_guard.py, which mention in.pcap only in prose. The tally was a proxy and should have been labelled as a weaker one than it was.