From 3ac00ae16d5f04a244d5eb95f56eefe05709db99 Mon Sep 17 00:00:00 2001 From: Arthit Suriyawongkul Date: Mon, 28 Sep 2026 11:49:53 +0100 Subject: [PATCH 1/2] fix: join autotune timer thread when training raises Signed-off-by: Arthit Suriyawongkul --- .../fasttext/tests/test_autotune_errors.py | 26 +++++++++++++++++++ src/autotune.cc | 9 +++++++ src/autotune.h | 2 +- 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 python/fasttext_module/fasttext/tests/test_autotune_errors.py diff --git a/python/fasttext_module/fasttext/tests/test_autotune_errors.py b/python/fasttext_module/fasttext/tests/test_autotune_errors.py new file mode 100644 index 0000000..2d12864 --- /dev/null +++ b/python/fasttext_module/fasttext/tests/test_autotune_errors.py @@ -0,0 +1,26 @@ +# SPDX-FileContributor: Arthit Suriyawongkul +# SPDX-FileCopyrightText: 2026-present, fasttext-community +# SPDX-FileType: SOURCE +# SPDX-License-Identifier: MIT + +"""Autotune errors must raise, not terminate the process.""" + +import pytest + +from .helpers import build_supervised_model, get_random_data + + +def test_autotune_error_raises(tmp_path): + data = get_random_data(3000, max_vocab_size=600) + valid = tmp_path / "valid.txt" + valid.write_text("".join(f"__label__{line}\n" for line in data)) + kwargs = { + # thread=12: thread <= 10 leaves the input matrix partly uninitialized. + "thread": 12, + "verbose": 0, + "autotuneValidationFile": str(valid), + "autotuneMetric": "f1:__label__missing", # fails after the first trial + "autotuneDuration": 60, # long enough for slow runners to reach it + } + with pytest.raises(RuntimeError, match="Unknown autotune metric label"): + build_supervised_model(data, kwargs) diff --git a/src/autotune.cc b/src/autotune.cc index 567731b..6ec44fb 100644 --- a/src/autotune.cc +++ b/src/autotune.cc @@ -215,6 +215,15 @@ Autotune::Autotune(const std::shared_ptr& fastText) strategy_(), timer_() {} +Autotune::~Autotune() noexcept { + // An exception leaving train() skips its timer join, and destroying a + // joinable std::thread calls std::terminate(). + if (timer_.joinable()) { + continueTraining_ = false; + timer_.join(); + } +} + void Autotune::printInfo(double maxDuration) { double progress = elapsed_ * 100 / maxDuration; progress = std::min(progress, 100.0); diff --git a/src/autotune.h b/src/autotune.h index 8b300ae..73e1405 100644 --- a/src/autotune.h +++ b/src/autotune.h @@ -81,7 +81,7 @@ class Autotune { Autotune(Autotune&&) = delete; Autotune& operator=(const Autotune&) = delete; Autotune& operator=(Autotune&&) = delete; - ~Autotune() noexcept = default; + ~Autotune() noexcept; void train(const Args& args); }; From 6c95779b3b3e94a43c112dd0cbc95795cedbb5ea Mon Sep 17 00:00:00 2001 From: Arthit Suriyawongkul Date: Mon, 28 Sep 2026 12:53:38 +0100 Subject: [PATCH 2/2] fix: restore SIGINT handler after autotune Signed-off-by: Arthit Suriyawongkul --- .../fasttext/tests/test_autotune_sigint.py | 57 +++++++++++++++++++ src/autotune.cc | 46 ++++++++++++++- 2 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 python/fasttext_module/fasttext/tests/test_autotune_sigint.py diff --git a/python/fasttext_module/fasttext/tests/test_autotune_sigint.py b/python/fasttext_module/fasttext/tests/test_autotune_sigint.py new file mode 100644 index 0000000..58a3632 --- /dev/null +++ b/python/fasttext_module/fasttext/tests/test_autotune_sigint.py @@ -0,0 +1,57 @@ +# SPDX-FileContributor: Arthit Suriyawongkul +# SPDX-FileCopyrightText: 2026-present, fasttext-community +# SPDX-FileType: SOURCE +# SPDX-License-Identifier: MIT + +"""Autotune must restore the SIGINT handler it replaced.""" + +import signal +import subprocess +import sys + +_SCRIPT = """ +import os, signal, tempfile, threading, time, fasttext +from fasttext.tests.helpers import get_random_data +data = get_random_data(3000, max_vocab_size=600) +with tempfile.NamedTemporaryFile("w", suffix=".txt", delete=False) as f: + f.write("".join(f"__label__{line}\\n" for line in data)) +# thread=12: thread <= 10 leaves the input matrix partly uninitialized. +fasttext.train_supervised( + f.name, autotuneValidationFile=f.name, autotuneDuration=2, thread=12, verbose=0 +) + +def later(delay, func, *args): + timer = threading.Timer(delay, func, args) + timer.daemon = True + timer.start() + +try: + signal.raise_signal(signal.SIGINT) + time.sleep(1) # KeyboardInterrupt is raised between bytecodes +except KeyboardInterrupt: + print("KeyboardInterrupt") + +if hasattr(signal, "pthread_kill"): + # Ctrl-C must also interrupt a blocking call (no SA_RESTART). + r, w = os.pipe() + later(0.5, signal.pthread_kill, threading.get_ident(), signal.SIGINT) + later(10, os.write, w, b"x") # unblocks the read if SIGINT did not + start = time.monotonic() + try: + os.read(r, 1) + except KeyboardInterrupt: + print("KeyboardInterrupt" if time.monotonic() - start < 5 else "late") +""" + + +def test_autotune_restores_sigint_handler(): + """Used to leave a handler pointing at the destroyed Autotune.""" + result = subprocess.run( + [sys.executable, "-c", _SCRIPT], + capture_output=True, + text=True, + timeout=120, + check=False, + ) + expected = ["KeyboardInterrupt"] * (2 if hasattr(signal, "pthread_kill") else 1) + assert result.stdout.split() == expected, result.stderr[-2000:] diff --git a/src/autotune.cc b/src/autotune.cc index 6ec44fb..e33e949 100644 --- a/src/autotune.cc +++ b/src/autotune.cc @@ -16,6 +16,10 @@ #include #include +#ifndef _WIN32 +#include +#endif + #define LOG_VAL(name, val) \ if (autotuneArgs.verbose > 2) { \ std::cout << #name " = " << val << std::endl; \ @@ -39,6 +43,39 @@ void signalHandler(int signal) { } } +// SIGINT disposition replaced by installSigint(). +// POSIX keeps the full sigaction: restoring with std::signal() adds +// SA_RESTART, so Python's Ctrl-C would no longer interrupt a blocking read. +#ifdef _WIN32 +void (*previousSigint)(int); +#else +struct sigaction previousSigint; +#endif +std::atomic sigintInstalled(false); + +void installSigint() { +#ifdef _WIN32 + previousSigint = std::signal(SIGINT, signalHandler); +#else + struct sigaction action = {}; + action.sa_handler = signalHandler; + action.sa_flags = SA_RESTART; // as std::signal(), for autotune's own I/O + sigemptyset(&action.sa_mask); + sigaction(SIGINT, &action, &previousSigint); +#endif + sigintInstalled = true; +} + +void restoreSigint() { + if (sigintInstalled.exchange(false)) { +#ifdef _WIN32 + std::signal(SIGINT, previousSigint); +#else + sigaction(SIGINT, &previousSigint, nullptr); +#endif + } +} + class ElapsedTimeMarker { std::chrono::steady_clock::time_point start_; @@ -216,6 +253,9 @@ Autotune::Autotune(const std::shared_ptr& fastText) timer_() {} Autotune::~Autotune() noexcept { + // Restore SIGINT: the installed handler calls into this object. + restoreSigint(); + interruptSignalHandler = nullptr; // An exception leaving train() skips its timer join, and destroying a // joinable std::thread calls std::terminate(). if (timer_.joinable()) { @@ -275,12 +315,12 @@ void Autotune::startTimer(const Args& args) { trials_ = 0; continueTraining_ = true; - auto previousSignalHandler = std::signal(SIGINT, signalHandler); - interruptSignalHandler = [&]() { - std::signal(SIGINT, previousSignalHandler); + interruptSignalHandler = [this]() { + restoreSigint(); std::cerr << std::endl << "Aborting autotune..." << std::endl; abort(); }; + installSigint(); } double Autotune::getMetricScore(