Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion sqlit/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,32 @@ def _sane_tty() -> None:
pass


def _prewarm_process_worker(runtime: RuntimeConfig) -> Any | None:
"""Spawn the process worker before the Textual App is constructed.

`multiprocessing.spawn` collects the parent's open file descriptors at
spawn time. Textual's `App.__init__` registers signal-handler pipes
and other non-inheritable FDs, which cause spawn to abort with
`ValueError: bad value(s) in fds_to_keep` (see issue #189). Spawning
here — before the App is constructed — is the latest point at which
the FD set is still clean.

Returns the live client, or None if spawn fails or the worker is
disabled. On failure we fall through to the lazy path inside the UI;
on macOS that path raises and the in-process executor takes over.
"""
if not runtime.process_worker:
return None
if runtime.mock.enabled:
return None
try:
from sqlit.domains.process_worker.app.process_worker_client import ProcessWorkerClient

return ProcessWorkerClient()
except Exception:
return None


def _run_app(app: Any) -> int:
exit_code: int | None = None
handled_signals = [signal.SIGINT, signal.SIGTERM]
Expand Down Expand Up @@ -843,10 +869,14 @@ def main() -> int:
print(f"Error: {exc}")
return 1

# Spawn the worker before the Textual App is constructed; see
# _prewarm_process_worker for why this matters on macOS.
process_worker_client = _prewarm_process_worker(runtime)
app = SSMSTUI(
services=services,
startup_connection=startup_config,
exclusive_connection=exclusive_connection,
process_worker_client=process_worker_client,
)
exit_code = _run_app(app)
if exit_code != 0:
Expand Down Expand Up @@ -907,7 +937,12 @@ def main() -> int:
print(f"Error: {alert_error}")
return 1

app = SSMSTUI(services=services, startup_connection=temp_config)
process_worker_client = _prewarm_process_worker(runtime)
app = SSMSTUI(
services=services,
startup_connection=temp_config,
process_worker_client=process_worker_client,
)
return _run_app(app)

if args.command in {"connections", "connection"}:
Expand Down
33 changes: 33 additions & 0 deletions sqlit/domains/process_worker/app/process_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,39 @@

from __future__ import annotations

import faulthandler
import os
import pickle
import sys
import tempfile
import threading
import time
from collections import deque
from dataclasses import dataclass, field
from multiprocessing.connection import Connection
from pathlib import Path
from typing import Any


def _open_worker_log() -> Any | None:
"""Open the worker log file, honoring SQLIT_WORKER_LOG when set.

The parent process may attach this worker to a Textual-managed pipe;
libpq notice writes or unexpected stderr output would then SIGPIPE the
child. Redirecting both streams to a real file avoids that and gives a
place to land C-level fault tracebacks for future diagnosis.
"""
override = os.environ.get("SQLIT_WORKER_LOG")
if override:
path = Path(override).expanduser()
else:
path = Path(tempfile.gettempdir()) / "sqlit-worker.log"
try:
path.parent.mkdir(parents=True, exist_ok=True)
return path.open("a", buffering=1)
except OSError:
return None

from sqlit.domains.connections.domain.config import ConnectionConfig
from sqlit.domains.connections.providers.catalog import get_provider
from sqlit.domains.connections.providers.config_service import normalize_connection_config
Expand Down Expand Up @@ -493,6 +518,14 @@ def _get_provider(self, db_type: str) -> Any | None:

def run_process_worker(conn: Connection) -> None:
"""Process entrypoint for query execution."""
log_file = _open_worker_log()
if log_file is not None:
sys.stdout = log_file
sys.stderr = log_file
try:
faulthandler.enable(file=log_file)
except (RuntimeError, ValueError):
pass
state = _WorkerState(conn=conn)
try:
while True:
Expand Down
8 changes: 8 additions & 0 deletions sqlit/domains/process_worker/app/process_worker_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ def _maybe_fallback_start(self, error: Exception) -> None:
raise error
if os.name != "posix" or sys.platform.startswith("win"):
raise error
# macOS (especially Tahoe 26+) aborts in the child whenever a forked
# process touches CoreFoundation / Security.framework / os_log —
# which happens inside psycopg, getaddrinfo, and many other stdlib
# paths. Forking here causes hard segfaults (issue #189), so we
# surface the spawn failure and let the caller fall back to
# in-process execution.
if sys.platform == "darwin":
raise error
try:
self._start_with_context(get_context("fork"))
except Exception:
Expand Down
7 changes: 7 additions & 0 deletions sqlit/domains/shell/app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,16 @@ def __init__(
runtime: RuntimeConfig | None = None,
startup_connection: ConnectionConfig | None = None,
exclusive_connection: bool = False,
process_worker_client: Any | None = None,
):
super().__init__()
self.services = services or build_app_services(runtime or RuntimeConfig.from_env())
# A pre-spawned worker handed in from cli.py, before Textual's
# FD set was contaminated. ProcessWorkerLifecycleMixin already
# checks self._process_worker_client first, so seeding it here
# short-circuits the lazy spawn path.
if process_worker_client is not None:
self._process_worker_client = process_worker_client
from sqlit.core.connection_manager import ConnectionManager

self._connection_manager = ConnectionManager(self.services)
Expand Down
64 changes: 64 additions & 0 deletions tests/unit/test_cli_prewarm.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
"""Tests for the pre-spawn helper in cli.py and the SSMSTUI kwarg plumbing."""

from __future__ import annotations

from unittest.mock import MagicMock, patch

from sqlit.cli import _prewarm_process_worker
from sqlit.shared.app.runtime import MockConfig, RuntimeConfig


def test_prewarm_skipped_when_worker_disabled() -> None:
runtime = RuntimeConfig(process_worker=False)
with patch("sqlit.domains.process_worker.app.process_worker_client.ProcessWorkerClient") as klass:
result = _prewarm_process_worker(runtime)
assert result is None
klass.assert_not_called()


def test_prewarm_skipped_when_mock_enabled() -> None:
runtime = RuntimeConfig(process_worker=True, mock=MockConfig(enabled=True))
with patch("sqlit.domains.process_worker.app.process_worker_client.ProcessWorkerClient") as klass:
result = _prewarm_process_worker(runtime)
assert result is None
klass.assert_not_called()


def test_prewarm_returns_client_when_enabled() -> None:
runtime = RuntimeConfig(process_worker=True)
sentinel = MagicMock(name="process-worker-client")
with patch(
"sqlit.domains.process_worker.app.process_worker_client.ProcessWorkerClient",
return_value=sentinel,
):
result = _prewarm_process_worker(runtime)
assert result is sentinel


def test_prewarm_swallows_spawn_errors() -> None:
"""A failing pre-spawn must not break startup; lazy path handles fallback."""
runtime = RuntimeConfig(process_worker=True)
with patch(
"sqlit.domains.process_worker.app.process_worker_client.ProcessWorkerClient",
side_effect=ValueError("bad value(s) in fds_to_keep"),
):
result = _prewarm_process_worker(runtime)
assert result is None


def test_ssmstui_stashes_prewarmed_worker() -> None:
"""A client passed via the kwarg should land on self._process_worker_client.

ProcessWorkerLifecycleMixin returns that attribute first, so this is
what makes the pre-spawn actually short-circuit the lazy path.
"""
from sqlit.domains.shell.app.main import SSMSTUI
from tests.ui.mocks import MockConnectionStore, MockSettingsStore, build_test_services, create_test_connection

services = build_test_services(
connection_store=MockConnectionStore([create_test_connection("test-db", "sqlite")]),
settings_store=MockSettingsStore({}),
)
sentinel = MagicMock(name="process-worker-client")
app = SSMSTUI(services=services, process_worker_client=sentinel)
assert app._process_worker_client is sentinel
68 changes: 68 additions & 0 deletions tests/unit/test_process_worker_fallback.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
"""Tests for the macOS fork-fallback guard and the worker log redirect."""

from __future__ import annotations

from pathlib import Path

import pytest

from sqlit.domains.process_worker.app.process_worker import _open_worker_log
from sqlit.domains.process_worker.app.process_worker_client import ProcessWorkerClient


def _make_client_without_init() -> ProcessWorkerClient:
"""Build a bare ProcessWorkerClient instance without running __init__.

__init__ spawns a real subprocess; we only want to exercise the
in-process branching logic in _maybe_fallback_start.
"""
return ProcessWorkerClient.__new__(ProcessWorkerClient)


def test_fork_fallback_disabled_on_darwin(monkeypatch: pytest.MonkeyPatch) -> None:
"""On macOS the fallback path must re-raise instead of forking."""
monkeypatch.setattr("sys.platform", "darwin")
client = _make_client_without_init()

error = ValueError("bad value(s) in fds_to_keep")
with pytest.raises(ValueError, match="fds_to_keep"):
client._maybe_fallback_start(error)


def test_fork_fallback_skips_non_fds_errors(monkeypatch: pytest.MonkeyPatch) -> None:
"""Errors unrelated to fds_to_keep should always re-raise verbatim."""
monkeypatch.setattr("sys.platform", "linux")
client = _make_client_without_init()

error = ValueError("totally unrelated message")
with pytest.raises(ValueError, match="totally unrelated"):
client._maybe_fallback_start(error)


def test_worker_log_honors_env_override(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
"""SQLIT_WORKER_LOG should redirect the worker log to a custom path."""
target = tmp_path / "subdir" / "worker.log"
monkeypatch.setenv("SQLIT_WORKER_LOG", str(target))

log_file = _open_worker_log()
assert log_file is not None
try:
log_file.write("hello\n")
finally:
log_file.close()

assert target.exists()
assert "hello" in target.read_text()


def test_worker_log_returns_none_on_unwritable_path(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""If the log path can't be opened, the worker should still boot."""
# A path whose parent can't be created (root-owned, non-existent).
monkeypatch.setenv("SQLIT_WORKER_LOG", "/proc/definitely/not/writable/here.log")

log_file = _open_worker_log()
assert log_file is None