Fix _kill_process AttributeError when _stats_logger is unset (#6417)
* Fix _kill_process AttributeError when _stats_logger is unset LlamaCppBackend._kill_process references self._stats_logger in its finally block, but __init__ only sets self._stats_logger = None partway through. If __init__ raises before that line, or the backend is built via __new__ (as the kill-path unit test does), teardown crashes with AttributeError instead of cleaning up the process. Guard the reference with getattr, matching the existing hasattr(self, '_chat_template_file') guard in the same finally block. Fixes test_kill_process_records_timestamp_on_actual_kill. * Also guard _stdout_thread in _kill_process teardown Review follow-up: the same finally block also reads self._stdout_thread, which is unset on a partially-built / __new__ backend. Guard it with getattr like _llama_log_fh below, and add a test that _kill_process tolerates a backend with those optional attrs unset. Trim the _stats_logger comment. --------- Co-authored-by: Michael Han <michaelhan2050@gmail.com>
This commit is contained in:
parent
88b1d6d496
commit
1b697ed6fc
2 changed files with 26 additions and 3 deletions
|
|
@ -6125,7 +6125,9 @@ class LlamaCppBackend:
|
|||
except Exception as e:
|
||||
logger.warning(f"Error killing llama-server process: {e}")
|
||||
finally:
|
||||
if self._stats_logger is not None:
|
||||
# getattr: teardown must tolerate a partially-built backend (failed
|
||||
# __init__ or a __new__-built instance), as with _llama_log_fh below.
|
||||
if getattr(self, "_stats_logger", None) is not None:
|
||||
self._stats_logger.stop()
|
||||
self._stats_logger = None
|
||||
self._process = None
|
||||
|
|
@ -6135,8 +6137,9 @@ class LlamaCppBackend:
|
|||
# Drives _wait_for_vram_settle in the next load_model; set in finally
|
||||
# so both in-process and frontend Apply paths record the kill.
|
||||
self._last_kill_monotonic = time.monotonic()
|
||||
if self._stdout_thread is not None:
|
||||
self._stdout_thread.join(timeout = 2)
|
||||
stdout_thread = getattr(self, "_stdout_thread", None)
|
||||
if stdout_thread is not None:
|
||||
stdout_thread.join(timeout = 2)
|
||||
self._stdout_thread = None
|
||||
fh = getattr(self, "_llama_log_fh", None)
|
||||
if fh is not None:
|
||||
|
|
|
|||
|
|
@ -309,6 +309,26 @@ def test_kill_process_records_timestamp_on_actual_kill():
|
|||
assert before <= backend._last_kill_monotonic <= after
|
||||
|
||||
|
||||
def test_kill_process_tolerates_partially_constructed_backend():
|
||||
# Teardown must not AttributeError on a __new__-built backend that never ran
|
||||
# __init__: _stats_logger / _stdout_thread / _llama_log_fh are left unset.
|
||||
backend = LlamaCppBackend.__new__(LlamaCppBackend)
|
||||
|
||||
class _FakeProcess:
|
||||
def terminate(self):
|
||||
pass
|
||||
|
||||
def wait(self, timeout = None):
|
||||
return 0
|
||||
|
||||
def kill(self):
|
||||
pass
|
||||
|
||||
backend._process = _FakeProcess()
|
||||
backend._kill_process()
|
||||
assert backend._process is None
|
||||
|
||||
|
||||
def test_helper_is_static_method_callable_off_class():
|
||||
"""Pin the @staticmethod binding so call sites can invoke off the class."""
|
||||
ctx, _state = _patch_probe([[]])
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue