fix(foundation): close only an owned input on leaving the with block (#1999)

CHANGELOG.md

@@ -139,7 +139,7 @@ Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` to `1.5.0b3` (all 2026-09-19) and
 
 - TCP reassembly mixed absolute sequence numbers with buffer-relative slicing, so on any capture carrying a SYN with a realistic initial sequence number incomplete datagrams were dropped silently, and the `completed=False` branch of the public API was unreachable ([#349](https://github.com/JarryShaw/PyPCAPKit/issues/349), [#376](https://github.com/JarryShaw/PyPCAPKit/pull/376)).
 - `format='text'` raised `AttributeError` before writing anything, naming a `dictdumper.Text` that has never existed. It now points at `Tree`, as the `'txt'` alias beside it already did.
-- `Extractor` had the ownership of its input stream inverted: a handle it opened itself, from `fin` given as a path, was **never closed**, while a stream the *caller* supplied and still needed **was**. `_cleanup` closed under `not self._flag_s` (the flag that gated the `open()`), and the `SeekableReader` wrapping a non-seekable input asked for `stream_closing=not self._flag_s` in the same wrong direction. The leak was one descriptor per extraction, the root cause of the `ResourceWarning` that reddened [#577](https://github.com/JarryShaw/PyPCAPKit/pull/577), [#596](https://github.com/JarryShaw/PyPCAPKit/pull/596) and [#600](https://github.com/JarryShaw/PyPCAPKit/pull/600) ([#606](https://github.com/JarryShaw/PyPCAPKit/issues/606) had only made the test immune). Both conditions now follow ownership via a single `Extractor._owns_input`, and a finaliser releases the handle of an extraction *abandoned* before end of file (`auto=False`, iterated part way and dropped); the files [#610](https://github.com/JarryShaw/PyPCAPKit/issues/610) names emit no such warning. `__exit__` deliberately still closes unconditionally, since a caller who scopes an `Extractor` with `with` asked for that ([#610](https://github.com/JarryShaw/PyPCAPKit/issues/610)).
+- `Extractor` had the ownership of its input stream inverted: a handle it opened itself, from `fin` given as a path, was **never closed**, while a stream the *caller* supplied and still needed **was**. `_cleanup` closed under `not self._flag_s` (the flag that gated the `open()`), and the `SeekableReader` wrapping a non-seekable input asked for `stream_closing=not self._flag_s` in the same wrong direction. The leak was one descriptor per extraction, the root cause of the `ResourceWarning` that reddened [#577](https://github.com/JarryShaw/PyPCAPKit/pull/577), [#596](https://github.com/JarryShaw/PyPCAPKit/pull/596) and [#600](https://github.com/JarryShaw/PyPCAPKit/pull/600) ([#606](https://github.com/JarryShaw/PyPCAPKit/issues/606) had only made the test immune). Both conditions now follow ownership via a single `Extractor._owns_input`, and a finaliser releases the handle of an extraction *abandoned* before end of file (`auto=False`, iterated part way and dropped); the files [#610](https://github.com/JarryShaw/PyPCAPKit/issues/610) names emit no such warning. Leaving a `with` block follows the same rule, so a caller's stream stays open after it ([#1999](https://github.com/JarryShaw/PyPCAPKit/issues/1999)).
 - `extract(..., no_eof=True)` never returned. End of stream was detected and `ExtractionWarning: EOF reached` fired, but every loop that handles it (`record_frames`, `__next__`, `__call__`) read `if self._flag_n: continue`, so the flag suppressed the error that *ended* the loop without supplying another ending and an exhausted input was retried forever. The flag is not pointless -- `__main__` sets `no_eof` for `fin='-'`, and the end of what has arrived on a live capture is not the end of the capture -- so this is a termination condition rather than a deletion. The input's own position tells a paused capture from a finished one: `prepare` raises end of stream when zero bytes remain and restores the position first, so two consecutive ends of stream at the same position mean nothing arrived between them (`Extractor._note_eof_progress`). A pipe is unaffected, since a read there *blocks* while the writer is open but idle. This also fixes `pcapkit -`, which hung on **any** finished stdin. **One deliberate narrowing**: a *seekable* input does not block, so a file still being appended to ends at the data present when the extraction reached it; a timed grace period would make the cut-off intermittent rather than absent, so following a growing file is left to a policy of its own. Pre-existing and untouched: an append landing *mid-record* raises `ValueError: read length must be non-negative or -1`. The end-to-end tests are bounded by a **child process** rather than `tests._support.time_limit`, because the in-process deadline proved intermittent on this loop, and an intermittent guard against a hang leaves a wedged suite rather than a red test. Guesses at the cause are ruled out: the parse path's `except Exception` handlers are never entered during the spin, and the memory is the retry loop's `EOF reached` records, which the runner retains ([#620](https://github.com/JarryShaw/PyPCAPKit/issues/620)).
 - `register_protocol` displaced one protocol class with another and said nothing. The registry behind `pcapkit.protocols.__proto__` is keyed on `cls.__name__.upper()`, and several dispatchable classes are named `HTTP` (the base `pcapkit.protocols.application.http.HTTP` and the `httpv1` and `httpv2` implementations), so a bare assignment decided the winner: `register_protocol(httpv2.HTTP)` moved `__proto__['HTTP']` to the `httpv2` class with no exception, log line or `RegistryWarning`, and `ProtocolBase.expand_comp('HTTP')` then resolved to whichever registered last. The overwrite now raises a `RegistryWarning` naming the displaced and the replacing class. The sharpest path is not a call to the registrar: `Protocol.__init_subclass__()` registers unconditionally, so merely *defining* a subclass of the public `Protocol` under a name a built-in holds displaced that built-in. A plain `import pcapkit` emits zero new warnings: the built-in `HTTP` classes derive from `ProtocolBase`, so `__init_subclass__` never fires for them, and import-time seeding keys off the distinct names `HTTP`, `HTTPv1` and `HTTPv2`. The guard deliberately reads "key present and the incumbent is a different class", not mere presence ([#726](https://github.com/JarryShaw/PyPCAPKit/pull/726) later gave its sibling registrars the same identity guard): this function is the funnel every wrapper registrar ends in, so registering one class under two codes (`register_tcp` then `register_udp`, a documented path) reaches it twice with nothing displaced. The collision is reported, not resolved: re-keying is a registry-format change that none of the bare-name readers can absorb (each degrades *silently* to a string or `Raw` on a miss), so it belongs to the registry redesign in [#514](https://github.com/JarryShaw/PyPCAPKit/issues/514). The issue called this the one registrar with no warning; in fact none warned by itself, the siblings only by delegating to a guarded classmethod ([#675](https://github.com/JarryShaw/PyPCAPKit/issues/675)).
 - the `#:` autodoc comments in `pcapkit/foundation/traceflow/traceflow.py` that named a bare `Type`, which Sphinx resolves against every class named `Type` in the project rather than `typing.Type`, and silently linked to `pcapkit.const.l2tp.type.Type`, an L2TP field-type enum; once [#684](https://github.com/JarryShaw/PyPCAPKit/issues/684) rendered `TraceFlow._foutio`, the built docs pointed at the wrong class with no warning. These sites now spell it `~typing.Type[Dumper]`, among the sites [#709](https://github.com/JarryShaw/PyPCAPKit/issues/709) lists. The other bare `Type` sites, in hand-written `:type:` fields under `docs/source/pcapkit/foundation/`, are fixed separately by [#714](https://github.com/JarryShaw/PyPCAPKit/pull/714) ([#709](https://github.com/JarryShaw/PyPCAPKit/issues/709)).

docs/source/changelog/1.5.0.rst

@@ -976,9 +976,8 @@ Fixed
   Both conditions now follow ownership via a single ``Extractor._owns_input``, and a
   finaliser releases the handle of an extraction *abandoned* before end of file
   (``auto=False``, iterated part way and dropped); the files :issue:`610` names
-  emit no such warning. ``__exit__`` deliberately still
-  closes unconditionally, since a caller who scopes an :class:`.Extractor` with ``with``
-  asked for that (:issue:`610`).
+  emit no such warning. Leaving a ``with`` block follows the same rule, so a caller's
+  stream stays open after it (:issue:`1999`).
 * ``extract(..., no_eof=True)`` never returned. End of stream was detected and
   ``ExtractionWarning: EOF reached`` fired, but every loop that handles it
   (``record_frames``, ``__next__``, ``__call__``) read ``if self._flag_n: continue``,

pcapkit/foundation/extraction.py

@@ -1590,9 +1590,26 @@ def __enter__(self) -> 'Extractor':
 
     def __exit__(self, exc_type: 'Type[BaseException] | None', exc_value: 'BaseException | None',
                  traceback: 'TracebackType | None') -> 'None':  # pylint: disable=unused-argument
-        """Close the input file when exits."""
+        """Release what this class holds when the ``with`` block exits.
+
+        The engine is always closed. The input stream is closed only *if this
+        class owns it* -- the rule of :issue:`610`, as :meth:`_owns_input` writes
+        it down and :meth:`_cleanup` and the backstops already follow. A stream
+        the caller supplied stays open for the caller to close, so leaving the
+        block no longer closes it behind the caller's back (:issue:`1999`).
+
+        For a caller's non-seekable stream, the stream this class owns is the
+        :class:`~pcapkit.corekit.io.SeekableReader` wrapped around it, and that
+        wrapper is closed; it was built with ``stream_closing`` off, so closing
+        it leaves the caller's stream underneath open.
+
+        """
         logger.debug('closing %s on context exit after %d frame(s)', self._ifnm, self._frnum)
-        self._ifile.close()
+        # NOTE: ownership, not the ``with`` statement, decides who closes the
+        # input (#610, #1999): entering the block does not hand the caller's
+        # stream over to this class.
+        if self._owns_input():
+            self._ifile.close()
         self._detach_finalizer()
         self._exeng.close()
 
@@ -1643,8 +1660,8 @@ def _owns_input(self) -> 'bool':
         """Whether the input stream is this class's to close.
 
         The one place the ownership rule of :issue:`610` is written down, so that
-        :meth:`_cleanup`, :meth:`__del__` and the registration of
-        :attr:`_finalizer` cannot drift apart on it.
+        :meth:`_cleanup`, :meth:`__exit__`, :meth:`__del__` and the registration
+        of :attr:`_finalizer` cannot drift apart on it.
 
         Returns:
             :data:`True` when the input stream was opened by this class and is

tests/_support.py

@@ -1135,8 +1135,9 @@ def close_extractor(extractor: object) -> None:
     :meth:`Extractor._cleanup <pcapkit.foundation.extraction.Extractor._cleanup>`
     or :meth:`Extractor.__exit__ <pcapkit.foundation.extraction.Extractor.__exit__>`,
     so nothing in the library closes up after them. Both of those close the
-    input file *and* the engine, and this helper has to do the same: the input
-    file is not the only resource. The ``pcap_ct`` and ``pypcap`` engines hold a
+    engine as well as an input file the extractor opened itself, and this helper
+    closes both, whoever opened the input: the input file is not the only
+    resource. The ``pcap_ct`` and ``pypcap`` engines hold a
     live :class:`pcap.pcap` handle, and ``pyshark`` holds a temporary file, so
     dropping the extractor without closing the engine leaks an OS-level handle
     per test. Under a suite that builds hundreds of extractors that accumulates

tests/foundation/test_extraction.py

@@ -949,11 +949,21 @@ def test_record_header_record_frames_iteration_call_and_cleanup(self) -> None:
         given_stream._cleanup()
         self.assertFalse(given_stream._ifile.closed_by_test)
 
+        # Leaving the ``with`` block follows the same ownership rule: it closes
+        # a handle ``Extractor`` opened, and leaves a caller's stream open. This
+        # assertion closed the caller's stream until #1999.
+        manual._flag_s = True
         with manual as ctx:
             self.assertIs(ctx, manual)
         self.assertTrue(manual._ifile.closed_by_test)
         self.assertGreaterEqual(manual._exeng.close_calls, 1)
 
+        scoped_stream = self._bare_extractor()
+        with scoped_stream:
+            pass
+        self.assertFalse(scoped_stream._ifile.closed_by_test)
+        self.assertGreaterEqual(scoped_stream._exeng.close_calls, 1)
+
     def test_constructor_initialises_without_running_when_patched(self) -> None:
         from pcapkit.foundation.extraction import Extractor
 

tests/foundation/test_extraction_ownership.py

@@ -337,6 +337,82 @@ def test_non_seekable_stream_survives_its_seekable_wrapper(self) -> None:
         stream.close()
 
 
+@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed')
+class ContextExitFollowsOwnership(unittest.TestCase):
+    """#1999: leaving a ``with`` block closes only what ``Extractor`` owns.
+
+    ``__exit__`` closed the input unconditionally, so scoping an extraction with
+    ``with`` closed a stream the caller had passed in and still held, which the
+    rule of #610 says is the caller's to close.
+
+    """
+
+    def test_callers_seekable_stream_stays_open(self) -> None:
+        """The issue's own repro: an ``auto=True`` extraction inside ``with``."""
+        import pcapkit
+
+        with open(sample_path('in.pcap'), 'rb') as handle:
+            with pcapkit.extract(fin=handle, nofile=True, store=False):
+                pass
+
+            with self.subTest(check='handle not closed'):
+                self.assertFalse(handle.closed,
+                                 "leaving the with block closed the caller's stream")
+
+            with self.subTest(check='handle still readable'):
+                handle.seek(0)
+                self.assertEqual(handle.read(4), b'\xd4\xc3\xb2\xa1')
+
+    def test_callers_non_seekable_stream_stays_open(self) -> None:
+        """The wrapper ``Extractor`` built is closed; the stream under it is not."""
+        import pcapkit
+        from pcapkit.corekit.io import SeekableReader
+
+        with open(sample_path('in.pcap'), 'rb') as source:
+            payload = source.read()
+
+        # The ``sys.stdin.buffer`` shape: see
+        # ``test_non_seekable_stream_survives_its_seekable_wrapper`` for why a
+        # bare non-seekable ``BytesIO`` will not do.
+        stream = io.BufferedReader(_NonSeekableRaw(payload))
+        self.addCleanup(stream.close)
+        with pcapkit.extract(fin=stream, nofile=True, store=False,
+                             auto=False) as extractor:
+            next(extractor)
+
+        self.assertIsInstance(extractor._ifile, SeekableReader,
+                              'a non-seekable input should have been wrapped, '
+                              'without which this test proves nothing')
+        with self.subTest(check='wrapper closed'):
+            self.assertTrue(extractor._ifile.closed,
+                            'leaving the with block should close the wrapper '
+                            'Extractor built')
+        with self.subTest(check="caller's stream still open"):
+            self.assertFalse(stream.closed,
+                             "leaving the with block closed the caller's stream "
+                             'through the wrapper')
+
+    def test_path_opened_handle_is_closed(self) -> None:
+        """A handle ``Extractor`` opened from a path is closed on leaving the block.
+
+        ``auto=False`` and one frame read, so the extraction is still short of end
+        of file and ``_cleanup`` has not run: the close is ``__exit__``'s alone.
+
+        """
+        import pcapkit
+
+        with pcapkit.extract(fin=sample_path('in.pcap'), nofile=True, store=False,
+                             auto=False) as extractor:
+            next(extractor)
+            self.assertFalse(extractor._flag_e,
+                             '_cleanup ran inside the block, so this test no '
+                             'longer measures __exit__')
+            self.assertFalse(extractor._ifile.closed)
+
+        self.assertTrue(extractor._ifile.closed,
+                        'leaving the with block left the handle Extractor opened')
+
+
 @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed')
 class FinaliserBackstopWithoutDel(unittest.TestCase):
     """#1997: the backstop is a :class:`weakref.finalize`, not ``__del__`` alone.