ipython · Carreau · Feb 15, 2025 · Feb 14, 2025
diff --git a/IPython/conftest.py b/IPython/conftest.py
@@ -81,6 +81,12 @@ def inject():
     from .core import page
 
     page.pager_page = nopage
+
+    from IPython.core.history import HistoryManager
+
+    # ensure we don't leak History managers
+    if os.name != "nt":
+        HistoryManager._max_inst = 1
     # yield
 
 

diff --git a/IPython/core/history.py b/IPython/core/history.py
@@ -14,6 +14,7 @@
 import threading
 from pathlib import Path
 
+from contextlib import contextmanager
 from decorator import decorator
 from traitlets import (
     Any,
@@ -35,6 +36,7 @@
 from typing import Iterable, Tuple, Optional, TYPE_CHECKING
 import typing
 from warnings import warn
+from weakref import ref, WeakSet
 
 if TYPE_CHECKING:
     from IPython.core.interactiveshell import InteractiveShell
@@ -620,7 +622,12 @@ def _dir_hist_default(self) -> typing.List[Path]:
 
     # History saving in separate thread
     save_thread = Instance("IPython.core.history.HistorySavingThread", allow_none=True)
-    save_flag = Instance(threading.Event, allow_none=False)
+
+    @property
+    def save_flag(self) -> threading.Event | None:
+        if self.save_thread is not None:
+            return self.save_thread.save_flag
+        return None
 
     # Private interface
     # Variables used to store the three last inputs from the user.  On each new
@@ -636,6 +643,9 @@ def _dir_hist_default(self) -> typing.List[Path]:
     # an exit call).
     _exit_re = re.compile(r"(exit|quit)(\s*\(.*\))?$")
 
+    _instances: WeakSet[HistoryManager] = WeakSet()
+    _max_inst: int | float = float("inf")
+
     def __init__(
         self,
         shell: InteractiveShell,
@@ -644,7 +654,6 @@ def __init__(
     ):
         """Create a new history manager associated with a shell instance."""
         super().__init__(shell=shell, config=config, **traits)
-        self.save_flag = threading.Event()
         self.db_input_cache_lock = threading.Lock()
         self.db_output_cache_lock = threading.Lock()
 
@@ -668,6 +677,15 @@ def __init__(
                     exc_info=True,
                 )
                 self.hist_file = ":memory:"
+        self._instances.add(self)
+        assert len(HistoryManager._instances) <= HistoryManager._max_inst, (
+            len(HistoryManager._instances),
+            HistoryManager._max_inst,
+        )
+
+    def __del__(self) -> None:
+        if self.save_thread is not None:
+            self.save_thread.stop()
 
     def _get_hist_file_name(self, profile: Optional[str] = None) -> Path:
         """Get default history file name based on the Shell's profile.
@@ -927,7 +945,8 @@ def store_inputs(
             self.db_input_cache.append((line_num, source, source_raw))
             # Trigger to flush cache and write to DB.
             if len(self.db_input_cache) >= self.db_cache_size:
-                self.save_flag.set()
+                if self.save_flag:
+                    self.save_flag.set()
 
         # update the auto _i variables
         self._iii = self._ii
@@ -959,7 +978,7 @@ def store_output(self, line_num: int) -> None:
 
         with self.db_output_cache_lock:
             self.db_output_cache.append((line_num, output))
-        if self.db_cache_size <= 1:
+        if self.db_cache_size <= 1 and self.save_flag is not None:
             self.save_flag.set()
 
     def _writeout_input_cache(self, conn: sqlite3.Connection) -> None:
@@ -1015,6 +1034,21 @@ def writeout_cache(self, conn: Optional[sqlite3.Connection] = None) -> None:
                 self.db_output_cache = []
 
 
+from typing import Callable, Iterator
+from weakref import ReferenceType
+
+
+@contextmanager
+def hold(ref: ReferenceType[HistoryManager]) -> Iterator[ReferenceType[HistoryManager]]:
+    """
+    Context manger that hold a reference to a weak ref to make sure it
+    is not GC'd during it's context.
+    """
+    r = ref()
+    yield ref
+    del r
+
+
 class HistorySavingThread(threading.Thread):
     """This thread takes care of writing history to the database, so that
     the UI isn't held up while that happens.
@@ -1023,32 +1057,43 @@ class HistorySavingThread(threading.Thread):
     the history cache. The main thread is responsible for setting the flag when
     the cache size reaches a defined threshold."""
 
-    daemon = True
-    stop_now = False
-    enabled = True
-    history_manager: HistoryManager
+    save_flag: threading.Event
+    daemon: bool = True
+    _stop_now: bool = False
+    enabled: bool = True
+    history_manager: ref[HistoryManager]
+    _stopped = False
 
     def __init__(self, history_manager: HistoryManager) -> None:
         super(HistorySavingThread, self).__init__(name="IPythonHistorySavingThread")
-        self.history_manager = history_manager
+        self.history_manager = ref(history_manager)
         self.enabled = history_manager.enabled
+        self.save_flag = threading.Event()
 
     @only_when_enabled
     def run(self) -> None:
         atexit.register(self.stop)
         # We need a separate db connection per thread:
         try:
-            self.db = sqlite3.connect(
-                str(self.history_manager.hist_file),
-                **self.history_manager.connection_options,
-            )
+            hm: ReferenceType[HistoryManager]
+            with hold(self.history_manager) as hm:
+                if hm() is not None:
+                    self.db = sqlite3.connect(
+                        str(hm().hist_file),  # type: ignore [union-attr]
+                        **hm().connection_options,  # type: ignore [union-attr]
+                    )
             while True:
-                self.history_manager.save_flag.wait()
-                if self.stop_now:
-                    self.db.close()
-                    return
-                self.history_manager.save_flag.clear()
-                self.history_manager.writeout_cache(self.db)
+                self.save_flag.wait()
+                with hold(self.history_manager) as hm:
+                    if hm() is None:
+                        self._stop_now = True
+                    if self._stop_now:
+                        self.db.close()
+                        return
+                    self.save_flag.clear()
+                    if hm() is not None:
+                        hm().writeout_cache(self.db)  # type: ignore [union-attr]
+
         except Exception as e:
             print(
                 (
@@ -1066,9 +1111,17 @@ def stop(self) -> None:
         Note that it does not attempt to write out remaining history before
         exiting. That should be done by calling the HistoryManager's
         end_session method."""
-        self.stop_now = True
-        self.history_manager.save_flag.set()
-        self.join()
+        if self._stopped:
+            return
+        self._stop_now = True
+
+        self.save_flag.set()
+        self._stopped = True
+        if self != threading.current_thread():
+            self.join()
+
+    def __del__(self) -> None:
+        self.stop()
 
 
 # To match, e.g. ~5/8-~2/3

diff --git a/IPython/core/interactiveshell.py b/IPython/core/interactiveshell.py
@@ -1333,14 +1333,15 @@ def init_user_ns(self):
         ns = {}
 
         # make global variables for user access to the histories
-        ns['_ih'] = self.history_manager.input_hist_parsed
-        ns['_oh'] = self.history_manager.output_hist
-        ns['_dh'] = self.history_manager.dir_hist
+        if self.history_manager is not None:
+            ns["_ih"] = self.history_manager.input_hist_parsed
+            ns["_oh"] = self.history_manager.output_hist
+            ns["_dh"] = self.history_manager.dir_hist
 
-        # user aliases to input and output histories.  These shouldn't show up
-        # in %who, as they can have very large reprs.
-        ns['In']  = self.history_manager.input_hist_parsed
-        ns['Out'] = self.history_manager.output_hist
+            # user aliases to input and output histories.  These shouldn't show up
+            # in %who, as they can have very large reprs.
+            ns["In"] = self.history_manager.input_hist_parsed
+            ns["Out"] = self.history_manager.output_hist
 
         # Store myself as the public api!!!
         ns['get_ipython'] = self.get_ipython
@@ -1377,8 +1378,8 @@ def reset(self, new_session=True, aggressive=False):
         If new_session is True, a new history session will be opened.
         """
         # Clear histories
-        assert self.history_manager is not None
-        self.history_manager.reset(new_session)
+        if self.history_manager is not None:
+            self.history_manager.reset(new_session)
         # Reset counter used to index all histories
         if new_session:
             self.execution_count = 1
@@ -3889,8 +3890,9 @@ def _atexit_once(self):
             # Close the history session (this stores the end time and line count)
             # this must be *before* the tempfile cleanup, in case of temporary
             # history db
-            self.history_manager.end_session()
-            self.history_manager = None
+            if self.history_manager is not None:
+                self.history_manager.end_session()
+                self.history_manager = None
 
     #-------------------------------------------------------------------------
     # Things related to IPython exiting

diff --git a/IPython/core/tests/test_history.py b/IPython/core/tests/test_history.py
@@ -7,6 +7,8 @@
 
 # stdlib
 import io
+import gc
+import os
 import sys
 import tempfile
 from datetime import datetime
@@ -18,16 +20,46 @@
 
 from IPython.core.history import HistoryAccessor, HistoryManager, extract_hist_ranges
 
+import pytest
 
 def test_proper_default_encoding():
     assert sys.getdefaultencoding() == "utf-8"
 
-def test_history():
+
+def hmmax_instance_maker(N: int):
+    if os.name == "nt":
+
+        @pytest.fixture()
+        def inner():
+            pass
+
+    else:
+
+        @pytest.fixture()
+        def inner():
+            assert HistoryManager._max_inst == 1
+            HistoryManager._max_inst = N
+            lh = len(HistoryManager._instances)
+            try:
+                yield
+                gc.collect()
+                assert len(HistoryManager._instances) == lh
+            finally:
+                HistoryManager._max_inst = 1
+
+    return inner
+
+
+hmmax2 = hmmax_instance_maker(2)
+hmmax3 = hmmax_instance_maker(3)
+
+
+def test_history(hmmax2):
     ip = get_ipython()
     with TemporaryDirectory() as tmpdir:
         tmp_path = Path(tmpdir)
         hist_manager_ori = ip.history_manager
-        hist_file = tmp_path / "history.sqlite"
+        hist_file = tmp_path / "history_test_history1.sqlite"
         try:
             ip.history_manager = HistoryManager(shell=ip, hist_file=hist_file)
             hist = ["a=1", "def f():\n    test = 1\n    return test", "b='€Æ¾÷ß'"]
@@ -145,10 +177,11 @@ def test_history():
             assert hist[1:] == (lineno, entry)
         finally:
             # Ensure saving thread is shut down before we try to clean up the files
-            ip.history_manager.save_thread.stop()
+            ip.history_manager.end_session()
             # Forcibly close database rather than relying on garbage collection
+            ip.history_manager.save_thread.stop()
             ip.history_manager.db.close()
-            # Restore history manager
+            # swap back
             ip.history_manager = hist_manager_ori
 
 
@@ -187,7 +220,7 @@ def test_timestamp_type():
     info = ip.history_manager.get_session_info()
     assert isinstance(info[1], datetime)
 
-def test_hist_file_config():
+def test_hist_file_config(hmmax3):
     cfg = Config()
     tfile = tempfile.NamedTemporaryFile(delete=False)
     cfg.HistoryManager.hist_file = Path(tfile.name)
@@ -202,8 +235,9 @@ def test_hist_file_config():
             # On Windows, even though we close the file, we still can't
             # delete it.  I have no clue why
             pass
+            HistoryManager.__max_inst = 1
 
-def test_histmanager_disabled():
+def test_histmanager_disabled(hmmax2):
     """Ensure that disabling the history manager doesn't create a database."""
     cfg = Config()
     cfg.HistoryAccessor.enabled = False
@@ -228,14 +262,14 @@ def test_histmanager_disabled():
     assert hist_file.exists() is False
 
 
-def test_get_tail_session_awareness():
+def test_get_tail_session_awareness(hmmax3):
     """Test .get_tail() is:
         - session specific in HistoryManager
         - session agnostic in HistoryAccessor
     same for .get_last_session_id()
     """
-    ip = get_ipython()
     with TemporaryDirectory() as tmpdir:
+        ip = get_ipython()
         tmp_path = Path(tmpdir)
         hist_file = tmp_path / "history.sqlite"
         get_source = lambda x: x[2]