Skip to content

Commit e04357b

Browse files
committed
fix(watch): handle Ctrl+C in MultiWatchProcessManager without signal handlers
signal.signal() raises ValueError when called from a worker thread, so multi-entry watch crashed under Django's autoreloader. Drop the handlers and rely on KeyboardInterrupt + finally cleanup, matching the single- entry path. Closes #201.
1 parent 5e1c254 commit e04357b

3 files changed

Lines changed: 80 additions & 24 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
## Unreleased
44

5+
### 🐛 Bug Fixes
6+
- **`tailwind watch` crash with `TAILWIND_CLI_CSS_MAP`**: `MultiWatchProcessManager` installed `signal.signal` handlers that fail with `ValueError: signal only works in main thread of the main interpreter` under Django's autoreloader (the watch loop runs in a worker thread). Cleanup now relies on `KeyboardInterrupt` propagation, matching the single-entry path. Fixes [#201](https://github.com/django-commons/django-tailwind-cli/issues/201).
7+
58
### 🔧 Technical Improvements
69
- **Hardened GitHub Actions workflows**: pinned all actions to commit SHAs, scoped top-level permissions, added concurrency groups, moved `github.ref_name` / `github.repository` out of shell interpolation into `env:` vars, and added a [zizmor](https://docs.zizmor.sh/) audit job to keep workflow security regressions out of CI.
710

src/django_tailwind_cli/management/commands/tailwind.py

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,10 +1130,9 @@ def start_watch_processes(self, config: Config, *, verbose: bool = False) -> Non
11301130
config: Configuration object with css_entries.
11311131
verbose: Whether to show detailed information.
11321132
"""
1133-
# Set up signal handlers for graceful shutdown
1134-
signal.signal(signal.SIGINT, self._signal_handler)
1135-
signal.signal(signal.SIGTERM, self._signal_handler)
1136-
1133+
# Rely on KeyboardInterrupt rather than signal.signal() so this works
1134+
# under Django's autoreloader, which runs the watch loop in a worker
1135+
# thread where signal.signal() raises ValueError.
11371136
try:
11381137
for entry in config.css_entries:
11391138
watch_cmd = config.get_watch_cmd(entry)
@@ -1153,19 +1152,17 @@ def start_watch_processes(self, config: Config, *, verbose: bool = False) -> Non
11531152
typer.secho(f"Watching '{entry.name}': {entry.src_css}", fg=typer.colors.GREEN)
11541153

11551154
self._monitor_processes()
1155+
except KeyboardInterrupt:
1156+
typer.secho("\nShutdown signal received, stopping watch processes...", fg=typer.colors.YELLOW)
1157+
self.shutdown_requested = True
11561158
except Exception as e:
11571159
typer.secho(f"Error starting watch processes: {e}", fg=typer.colors.RED)
1158-
self._cleanup_processes()
11591160
raise
1160-
1161-
def _signal_handler(self, _signum: int, _frame: FrameType | None) -> None:
1162-
"""Handle shutdown signals gracefully."""
1163-
typer.secho("\nShutdown signal received, stopping watch processes...", fg=typer.colors.YELLOW)
1164-
self.shutdown_requested = True
1165-
self._cleanup_processes()
1161+
finally:
1162+
self._cleanup_processes()
11661163

11671164
def _monitor_processes(self) -> None:
1168-
"""Monitor all watch processes."""
1165+
"""Monitor all watch processes. Cleanup is owned by start_watch_processes' finally."""
11691166
while not self.shutdown_requested and any(p.poll() is None for p in self.processes):
11701167
time.sleep(0.5)
11711168

@@ -1175,8 +1172,6 @@ def _monitor_processes(self) -> None:
11751172
self.shutdown_requested = True
11761173
break
11771174

1178-
self._cleanup_processes()
1179-
11801175
def _cleanup_processes(self) -> None:
11811176
"""Clean up all watch processes."""
11821177
for process in self.processes:

tests/test_integration.py

Lines changed: 68 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import os
99
import platform
10+
import threading
1011
import time
1112
from pathlib import Path
1213
from collections.abc import Callable
@@ -25,6 +26,7 @@
2526
DAISY_UI_SOURCE_CSS,
2627
DEFAULT_SOURCE_CSS,
2728
ProcessManager,
29+
_run_watch_loop,
2830
)
2931

3032

@@ -33,6 +35,13 @@ def _call_directly(func: Any, *args: Any, **kwargs: Any) -> Any:
3335
return func(*args, **kwargs)
3436

3537

38+
def _clear_legacy_css_settings(settings: LazySettings) -> None:
39+
"""Drop single-file CSS settings so CSS_MAP is the sole source of truth."""
40+
for name in ("TAILWIND_CLI_SRC_CSS", "TAILWIND_CLI_DIST_CSS"):
41+
if hasattr(settings, name):
42+
delattr(settings, name)
43+
44+
3645
class TestBuildWorkflowIntegration:
3746
"""Test complete build workflow from setup to CSS generation."""
3847

@@ -197,11 +206,7 @@ def test_build_with_multiple_css_entries(self, settings: LazySettings, tmp_path:
197206
("admin.css", "admin.output.css"),
198207
("web.css", "web.output.css"),
199208
]
200-
# Remove single-file settings to avoid conflict
201-
if hasattr(settings, "TAILWIND_CLI_SRC_CSS"):
202-
delattr(settings, "TAILWIND_CLI_SRC_CSS")
203-
if hasattr(settings, "TAILWIND_CLI_DIST_CSS"):
204-
delattr(settings, "TAILWIND_CLI_DIST_CSS")
209+
_clear_legacy_css_settings(settings)
205210

206211
# Create source CSS files
207212
(tmp_path / "admin.css").write_text('@import "tailwindcss";')
@@ -308,11 +313,7 @@ def test_watch_with_multiple_css_entries(self, settings: LazySettings, tmp_path:
308313
("admin.css", "admin.output.css"),
309314
("web.css", "web.output.css"),
310315
]
311-
# Remove single-file settings to avoid conflict
312-
if hasattr(settings, "TAILWIND_CLI_SRC_CSS"):
313-
delattr(settings, "TAILWIND_CLI_SRC_CSS")
314-
if hasattr(settings, "TAILWIND_CLI_DIST_CSS"):
315-
delattr(settings, "TAILWIND_CLI_DIST_CSS")
316+
_clear_legacy_css_settings(settings)
316317

317318
# Create source CSS files
318319
(tmp_path / "admin.css").write_text('@import "tailwindcss";')
@@ -361,6 +362,63 @@ def mock_download_func(
361362
assert "web.output.css" in str(call_args_1)
362363
assert "--watch" in call_args_1
363364

365+
def test_watch_with_css_map_runs_in_worker_thread(self, settings: LazySettings, tmp_path: Path):
366+
"""Regression for #201: multi-entry watch must work outside the main thread.
367+
368+
Django's autoreload.run_with_reloader executes the wrapped callable
369+
in a worker thread. signal.signal() raises ValueError there, so the
370+
multi-entry watch path must not rely on signal handlers.
371+
"""
372+
settings.BASE_DIR = tmp_path
373+
settings.TAILWIND_CLI_PATH = tmp_path / ".django_tailwind_cli"
374+
settings.STATICFILES_DIRS = (tmp_path / "assets",)
375+
settings.TAILWIND_CLI_VERSION = "4.1.3"
376+
settings.TAILWIND_CLI_CSS_MAP = [
377+
("admin.css", "admin.output.css"),
378+
("web.css", "web.output.css"),
379+
]
380+
_clear_legacy_css_settings(settings)
381+
382+
(tmp_path / "admin.css").write_text('@import "tailwindcss";')
383+
(tmp_path / "web.css").write_text('@import "tailwindcss";')
384+
385+
with (
386+
patch("django_tailwind_cli.utils.http.download_with_progress") as mock_download,
387+
patch("subprocess.Popen") as mock_popen,
388+
):
389+
390+
def mock_download_func(
391+
url: str,
392+
filepath: Path,
393+
timeout: int = 30,
394+
progress_callback: Callable[[int, int, float], None] | None = None,
395+
) -> None:
396+
filepath.parent.mkdir(parents=True, exist_ok=True)
397+
filepath.write_bytes(b"fake-cli-binary")
398+
filepath.chmod(0o755)
399+
400+
mock_download.side_effect = mock_download_func
401+
402+
mock_process = Mock()
403+
mock_process.poll.return_value = 0 # already exited → monitor loop returns immediately
404+
mock_process.wait.return_value = 0
405+
mock_popen.return_value = mock_process
406+
407+
errors: list[BaseException] = []
408+
409+
def runner() -> None:
410+
try:
411+
_run_watch_loop(verbose=False)
412+
except BaseException as exc:
413+
errors.append(exc)
414+
415+
worker = threading.Thread(target=runner)
416+
worker.start()
417+
worker.join(timeout=5)
418+
419+
assert not worker.is_alive(), "_run_watch_loop did not terminate within 5s"
420+
assert not errors, f"_run_watch_loop crashed in worker thread: {errors[0]!r}"
421+
364422
def test_watch_keyboard_interrupt_handling(
365423
self, settings: LazySettings, tmp_path: Path, capsys: CaptureFixture[str]
366424
):

0 commit comments

Comments
 (0)