mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-13 02:44:21 +00:00
* Fix studio server crash when launched without a console (Windows)
A hidden Windows subprocess (CREATE_NO_WINDOW, e.g. `unsloth studio` launched
from install.ps1 or a launcher) has no console, so the interpreter leaves
sys.stdout / sys.stderr as None. `_setup_server_disk_logging` wraps them in a
_TeeStream, and write()/flush()/close() delegated straight to the wrapped
stream, crashing on the first print with:
AttributeError: 'NoneType' object has no attribute 'write'
Make the tee tolerate a None wrapped stream: write() no-ops (still logging to
the session file), flush()/close() no-op, and __getattr__ raises AttributeError
instead of crashing. `_harden_console_close` also early-returns on None.
Adds an AST regression test (pinned the same way as the other studio tests
because importing run.py needs the full studio venv).
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
* Normalize missing std streams so the console-less launch actually starts
The _TeeStream guards land too late to fix the reported crash. run.py imports
loggers (and so structlog) at line 109, and structlog binds `from sys import
stdout` at its own import time, so a None stdout is captured permanently before
run_server() begins. The first logger.info() then dies with "cannot create weak
reference to 'NoneType' object", 36 lines before _setup_server_disk_logging()
installs the tee. Even past that, uvicorn.Config() probes sys.stdout.isatty()
and LOGGING_CONFIG leaves use_colors as None, so startup aborts anyway. And
UNSLOTH_STUDIO_NO_FILE_LOG=1 skips the tee entirely, so the guards never apply.
Point the missing streams at the null device at the top of run.py instead, before
the loggers import. A null-device stream answers encoding/buffer/fileno/isatty/
reconfigure like a real one, so structlog, uvicorn and the __main__ failure
handler all work and nothing downstream needs its own None check. Streams that
already exist are left untouched by identity, so this is a no-op on Linux, macOS,
Colab (ipykernel OutStream), Tauri (piped stdout) and pytest capture.
Also pass our std handles to the backend on Windows: without them
CREATE_NO_WINDOW gives the child its own hidden console, so `unsloth studio > log`
captures nothing. This mirrors what the setup.ps1 call already does.
Tests: the AST helper accepted any `if ... is None` anywhere in the method, so it
passed on a _TeeStream that still raised AttributeError on the first write. It now
requires an early-exit guard naming the wrapped stream, plus the ordering
constraint above. Real behavioural coverage goes in
studio/backend/tests/test_server_disk_logging.py, which runs on Python 3.10-3.13
rather than tests/studio/ on 3.12 only.
Verified on Python 3.9-3.14 x structlog {24.1.0, 26.1.0} x uvicorn {0.51.0,
0.52.1}, and on windows-latest / macos-14 / ubuntu-latest. On real Windows,
DETACHED_PROCESS and pythonw.exe are what leave the streams as None;
CREATE_NO_WINDOW alone gives the child a hidden console with valid handles, so
the comments now say that instead.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Tighten the comments added by this PR
Comment and docstring text only, no code change. Keeps the load-bearing facts:
the ordering constraint against the loggers import, why the launcher hands its
std handles to the child, and why _guards_target is strict.
* Let the guard check see through a local alias
_guards_target only matched a guard that named the attribute directly, so the
idiomatic `stream = self._stream; if stream is None: return` refactor failed the
test even though it is correct. Track assignments whose value is the target and
accept a guard on either name. Still rejects a guard on the wrong object, a
guard with no early exit, and an alias of a different attribute.
* Correct two rationales in the comments
Omitting stdin from the Popen call does not withhold it: subprocess fills
hStdInput from GetStdHandle(STD_INPUT_HANDLE) whenever any other handle is
passed, so the old comment described a guarantee the code does not make.
State what the sys.__stdout__ assignment buys instead of asserting a generic
fallback: rich reads sys.__stdout__.fileno() at import and otherwise settles on
fds 0/1/2, which a process with no std handles does not have.
---------
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Co-authored-by: Daniel Han <unslothai@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
155 lines
6.3 KiB
Python
155 lines
6.3 KiB
Python
# SPDX-License-Identifier: AGPL-3.0-only
|
|
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
|
|
|
|
"""Tests for the server session log + native-crash capture in run.py.
|
|
|
|
Field regression: Unsloth "terminates without a warning" -- a native crash in
|
|
the GPU runtime kills the process with no Python traceback, and a desktop-
|
|
shortcut console closes before anything can be read. The server must tee its
|
|
console output to disk and aim faulthandler at the same file so even hard
|
|
crashes leave evidence.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import io
|
|
import sys
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
_BACKEND_DIR = str(Path(__file__).resolve().parent.parent)
|
|
if _BACKEND_DIR not in sys.path:
|
|
sys.path.insert(0, _BACKEND_DIR)
|
|
|
|
import run as run_mod # noqa: E402
|
|
|
|
|
|
class TestTeeStream:
|
|
def test_writes_reach_both_and_return_original(self):
|
|
console, log = io.StringIO(), io.StringIO()
|
|
tee = run_mod._TeeStream(console, log)
|
|
n = tee.write("hello")
|
|
assert console.getvalue() == "hello" == log.getvalue()
|
|
assert n == 5 # delegate's return value, console contract unchanged
|
|
|
|
def test_log_failure_never_breaks_console(self):
|
|
class Broken:
|
|
def write(self, data):
|
|
raise OSError("disk full")
|
|
|
|
def flush(self):
|
|
raise OSError("disk full")
|
|
|
|
console = io.StringIO()
|
|
tee = run_mod._TeeStream(console, Broken())
|
|
assert tee.write("still works") == len("still works")
|
|
tee.flush() # must not raise
|
|
assert console.getvalue() == "still works"
|
|
|
|
def test_attribute_proxy(self):
|
|
console, log = io.StringIO(), io.StringIO()
|
|
tee = run_mod._TeeStream(console, log)
|
|
# isatty / encoding probes must see the original stream's answers.
|
|
assert tee.isatty() == console.isatty()
|
|
|
|
def test_missing_console_is_a_null_sink(self):
|
|
# Production never builds this, but _TeeStream(None, ...) must not crash.
|
|
log = io.StringIO()
|
|
tee = run_mod._TeeStream(None, log)
|
|
assert tee.write("hello") == len("hello") # text-stream write contract
|
|
tee.flush()
|
|
tee.close()
|
|
assert log.getvalue() == "hello"
|
|
assert not log.closed # the tee does not own the log handle
|
|
run_mod._harden_console_close(None) # must not raise
|
|
|
|
|
|
class TestNormalizeStandardStreams:
|
|
"""A Windows process with no valid std handles starts with them all None."""
|
|
|
|
def test_missing_streams_become_usable_text_streams(self, monkeypatch):
|
|
for name in ("stdin", "stdout", "stderr"):
|
|
monkeypatch.setattr(sys, name, None)
|
|
monkeypatch.setattr(sys, f"__{name}__", None)
|
|
run_mod._normalize_standard_streams()
|
|
try:
|
|
for name in ("stdin", "stdout", "stderr"):
|
|
stream = getattr(sys, name)
|
|
assert stream is not None
|
|
assert getattr(sys, f"__{name}__") is not None
|
|
# uvicorn's default formatter probes isatty(); logging needs write().
|
|
assert stream.isatty() is False
|
|
assert stream.encoding
|
|
assert stream.fileno() >= 0
|
|
sys.stdout.write("discarded")
|
|
sys.stdout.flush()
|
|
print("also discarded")
|
|
finally:
|
|
for name in ("stdin", "stdout", "stderr"):
|
|
stream = getattr(sys, name)
|
|
if stream is not None:
|
|
stream.close()
|
|
|
|
def test_existing_streams_are_left_alone(self, monkeypatch):
|
|
console = io.StringIO()
|
|
monkeypatch.setattr(sys, "stdout", console)
|
|
monkeypatch.setattr(sys, "stderr", console)
|
|
run_mod._normalize_standard_streams()
|
|
# Identity, not truthiness: replacing a live console would break Colab
|
|
# (ipykernel OutStream), Tauri's stdout protocol and pytest capture.
|
|
assert sys.stdout is console
|
|
assert sys.stderr is console
|
|
|
|
def test_runs_before_the_logger_import(self):
|
|
# structlog binds `from sys import stdout` at import time, so normalizing
|
|
# after the loggers import leaves None captured forever.
|
|
src = (Path(_BACKEND_DIR) / "run.py").read_text(encoding = "utf-8")
|
|
call = "\n_normalize_standard_streams()"
|
|
assert call in src, "run.py never calls _normalize_standard_streams()"
|
|
assert src.index(call) < src.index("\nfrom loggers import get_logger")
|
|
|
|
|
|
class TestSetupServerDiskLogging:
|
|
def test_opt_out_env(self, monkeypatch):
|
|
monkeypatch.setenv("UNSLOTH_STUDIO_NO_FILE_LOG", "1")
|
|
assert run_mod._setup_server_disk_logging() is None
|
|
|
|
def test_creates_log_and_enables_faulthandler(self, monkeypatch, tmp_path):
|
|
import faulthandler
|
|
|
|
monkeypatch.delenv("UNSLOTH_STUDIO_NO_FILE_LOG", raising = False)
|
|
monkeypatch.delenv("PYTHONFAULTHANDLER", raising = False)
|
|
# Both resolution paths (utils.paths.studio_root and the env
|
|
# fallback) honor UNSLOTH_STUDIO_HOME, so this redirects the log dir.
|
|
monkeypatch.setenv("UNSLOTH_STUDIO_HOME", str(tmp_path))
|
|
orig_out, orig_err = sys.stdout, sys.stderr
|
|
was_enabled = faulthandler.is_enabled()
|
|
try:
|
|
log_path = run_mod._setup_server_disk_logging()
|
|
assert log_path is not None
|
|
assert Path(log_path).is_file()
|
|
assert "logs" in str(log_path)
|
|
# faulthandler armed at the file; children inherit the env switch.
|
|
assert faulthandler.is_enabled()
|
|
import os
|
|
|
|
assert os.environ.get("PYTHONFAULTHANDLER") == "1"
|
|
print("tee-capture-marker")
|
|
sys.stdout.flush()
|
|
assert "tee-capture-marker" in Path(log_path).read_text(
|
|
encoding = "utf-8", errors = "replace"
|
|
)
|
|
finally:
|
|
sys.stdout, sys.stderr = orig_out, orig_err
|
|
if not was_enabled:
|
|
faulthandler.disable()
|
|
|
|
def test_run_server_wires_logging_before_main_import(self):
|
|
src = (Path(_BACKEND_DIR) / "run.py").read_text(encoding = "utf-8")
|
|
call_idx = src.index("_setup_server_disk_logging()", src.index("def run_server"))
|
|
main_import_idx = src.index("from main import app", src.index("def run_server"))
|
|
assert call_idx < main_import_idx, (
|
|
"disk logging must be armed before importing main so import-time "
|
|
"failures leave evidence on disk"
|
|
)
|