[JarryShaw]

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort)
• make test passes, and a test case covers the change
• Added a changelog entry — N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

• fix — bug fixes

---

Description of your pull request and other information

Closes #2107. Before this, when an engine raised out of the Extractor constructor after _exeng was set, #2106's handler closed the input but never the engine. PyShark's _extmp capture, its temp file and its tshark stayed open until GC.

• Fix: after closing the owned input, the handler takes self.__dict__.get('_exeng') and, if one exists, calls close() inside try: … except Exception: (logged at debug), then does a bare raise. The input is closed first and the engine second, the same order as _cleanup and __exit__. A failing close cannot replace the original exception, and Exception rather than BaseException lets a Ctrl-C during close through.

• PyShark: its close() raised AttributeError before run(), when _extmp is still None. It now skips that case. Every built-in engine's close() is now safe on a half-started engine and when called twice; the KeyboardInterrupt path already closes once through _cleanup.

• Test: test_extraction_constructor_closes_engine_2107_unit (4 cases):

  • a fake engine whose run() raises: close() ran, and the same exception object propagated;
  • a read_frame() that raises: the same checks;
  • a close() that raises UnsupportedCall: the original FormatError still wins;
  • PyShark's early close() is now a no-op.

  With the source reverted, all 4 fail. The fix(foundation): Extractor keeps its input open when the constructor's run() raises #2100, ownership, extraction, non-seekable and engine tests pass.

[JarryShaw]

GOOD TO GO at 3ec29479b (Sonnet review).

• Original error propagates: the nested except Exception around engine.close() restores the outer exception, so the bare raise re-raises the original. The new test asserts raised is original and __context__ is None when close() raises.
• Engine lookup: _exeng is a plain instance attribute (it has only a class annotation), so self.__dict__.get('_exeng') is correct. It is None for an unknown format.
• Double close is safe: PyPCAP/PCAP_CT guard with _closed and a None handle, and the other engines inherit the no-op close(). PyShark sets _extmp only as run()'s last statement, so skipping close() while it is None leaks nothing.
• Test: with extraction.py and pyshark.py reverted, all 4 cases fail. The fix(foundation): Extractor keeps its input open when the constructor's run() raises #2100, ownership, extraction and engine-stub tests pass.
• Known edge, no change: a BaseException from close(), such as a second Ctrl-C, can replace the original error.

[github-actions]

Coverage: 91.55% (unit tier, Python 3.15, 972f5bcd2, Unit Tests run success)

Package	Statements	Missed	Branches	Partial	Cover
pcapkit (top level)	104	4	20	2	95.16%
pcapkit/const	19599	880	2618	866	92.12%
pcapkit/corekit	2589	75	816	37	96.48%
pcapkit/dumpkit	282	3	102	3	98.44%
pcapkit/foundation	2969	23	1036	25	98.80%
pcapkit/interface	138	4	52	3	96.32%
pcapkit/protocols	17418	219	4574	185	98.14%
pcapkit/toolkit	869	36	298	10	95.20%
pcapkit/utilities	498	7	144	5	98.13%
pcapkit/vendor	4432	1988	1018	167	51.87%

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

[JarryShaw]

Ready to merge at 3ec29479b.

• Review: the Sonnet review is GOOD TO GO.
• CI: every blocking check passed (Required checks passed SUCCESS). The 3.8–3.10 KTLO legs carry no warnings.
• Ordering: the tests/foundation plain-unittest ordering leg passed on a scratch merge into main (82b901b91). It ran 723 tests with 0 failures.

This closes #2107.

[JarryShaw]

In this case, I would rather prefer to revise the Extractor's built-in close (or cleanup? i forgot its name) method. and directly call that here in __init__

[JarryShaw]

Understood: the constructor should reuse the Extractor's own cleanup path rather than carry a second, hand-rolled one.

Today that path is split in two:

• _cleanup closes the input this class owns, detaches the finalizer, calls TraceFlow.finish() and closes _exeng.
• __exit__ does the same minus the trace finish.

The plan:

• Revise _cleanup so it is safe to call from a constructor that failed partway. That means no engine when the format was unknown, and no trace finish for a trace that never ran.
• The except BaseException block in __init__ then calls self._cleanup() directly, still re-raising the original error if the engine's close raises.
• __exit__ shares the same helper.

I'm dispatching the revision on this branch now. It will come back with a fresh review.

[JarryShaw]

GOOD TO GO at 972f5bcd2 (Sonnet review of the rework, rebased onto main 23a815b84).

• As asked: the constructor's except BaseException is now self._cleanup(failed=True) followed by raise. _cleanup and __exit__ share one close path, the new _release(), which is documented.
• What failed=True changes:
  • Skips TCP.finish() and _flag_e, so flows of a capture that errored are not reported as ended.
  • Skips a missing _exeng, as with an unknown format.
  • Logs, rather than raises, an Exception from the engine's close(), so the constructor's error is the one reported.
• Unchanged: __exit__ and the normal end-of-file _cleanup() keep the same order and the same propagation. The KeyboardInterrupt paths still finish the trace once; the constructor's later second close is safe.
• Rebase fix: after perf(foundation): probe the pcap backend once per extraction (#2113) #2121, the PR's original tests failed with AttributeError: '_exprb', because they bypass _select_engine. The fake now sets up the _pcap_backend.Memo.
• Tests: without the change, 6 of the 8 tests fail and 3 error. With it, all 8 pass. The foundation ordering leg gives 730 tests, 0 failures.
• Nits, not blocking:
  • An OSError from closing the owned input would still replace the original error and skip the engine close. That was true for the input before this PR.
  • A BaseException from close() still propagates, which the docstring only implies.

[JarryShaw]

Ready to merge at 972f5bcd2.

• Review: the Sonnet review of the rework is GOOD TO GO.
• CI: every blocking check passed (Required checks passed SUCCESS). The 3.8–3.10 KTLO legs carry no warnings.
• Ordering: the head sits directly on main 23a815b84. Its tests/foundation plain-unittest ordering leg ran 730 tests with 0 failures, both in the worker's run and independently in the reviewer's.

This closes #2107.