From b546dff30266f647be1238943322ca309a4d3948 Mon Sep 17 00:00:00 2001 From: Andy C Date: Tue, 29 Oct 2024 13:51:25 -0400 Subject: [PATCH] [refactor] Move signal handling from cpp/ -> mycpp/iolib Because mylib::Stdin()->readline() needs to raise KeyboardInterrupt! Signal handling is inherently part of the mycpp runtime. --- build/dynamic-deps.sh | 1 + builtin/trap_osh.py | 10 +- core/comp_ui.py | 4 +- core/process.py | 25 +-- core/pyos.py | 160 +------------------ core/shell.py | 7 +- cpp/core.cc | 46 +----- cpp/core.h | 162 ------------------- cpp/core_test.cc | 94 +---------- cpp/frontend_pyreadline.cc | 3 +- frontend/reader.py | 10 +- mycpp/NINJA_subgraph.py | 11 +- mycpp/gc_iolib.cc | 46 ++++++ mycpp/gc_iolib.h | 175 +++++++++++++++++++++ mycpp/gc_iolib_test.cc | 109 +++++++++++++ mycpp/gc_mylib.cc | 9 ++ mycpp/iolib.py | 165 +++++++++++++++++++ mycpp/mylib.py | 3 +- mycpp/runtime.h | 1 + osh/cmd_eval.py | 3 +- pea/oils-typecheck.txt | 2 + prebuilt/dynamic-deps/filter-translate.txt | 1 + 22 files changed, 559 insertions(+), 488 deletions(-) create mode 100644 mycpp/gc_iolib.cc create mode 100644 mycpp/gc_iolib.h create mode 100644 mycpp/gc_iolib_test.cc create mode 100644 mycpp/iolib.py diff --git a/build/dynamic-deps.sh b/build/dynamic-deps.sh index c6db803ae..3e12d602e 100755 --- a/build/dynamic-deps.sh +++ b/build/dynamic-deps.sh @@ -83,6 +83,7 @@ frontend/py.*\.py # py_readline.py ported by hand to C++ frontend/consts.py # frontend/consts_gen.py frontend/match.py # frontend/lexer_gen.py +mycpp/iolib.py # Implemented in gc_iolib.{h,cC} mycpp/mops.py # Implemented in gc_mops.{h,cC} pgen2/grammar.py # These files are re-done in C++ diff --git a/builtin/trap_osh.py b/builtin/trap_osh.py index 4a5201015..192e87ba8 100644 --- a/builtin/trap_osh.py +++ b/builtin/trap_osh.py @@ -10,12 +10,12 @@ from core import dev from core import error from core import main_loop -from core import pyos from core import vm from frontend import flag_util from frontend import match from frontend import reader from frontend import signal_def +from mycpp import iolib from mycpp import mylib from mycpp.mylib import iteritems, print_stderr, log from mycpp import mops @@ -42,7 +42,7 @@ class TrapState(object): """ def __init__(self, signal_safe): - # type: (pyos.SignalSafe) -> None + # type: (iolib.SignalSafe) -> None self.signal_safe = signal_safe self.hooks = {} # type: Dict[str, command_t] self.traps = {} # type: Dict[int, command_t] @@ -88,7 +88,7 @@ def AddUserTrap(self, sig_num, handler): elif sig_num == SIGWINCH: self.signal_safe.SetSigWinchCode(SIGWINCH) else: - pyos.RegisterSignalInterest(sig_num) + iolib.RegisterSignalInterest(sig_num) def RemoveUserTrap(self, sig_num): # type: (int) -> None @@ -99,7 +99,7 @@ def RemoveUserTrap(self, sig_num): self.signal_safe.SetSigIntTrapped(False) pass elif sig_num == SIGWINCH: - self.signal_safe.SetSigWinchCode(pyos.UNTRAPPED_SIGWINCH) + self.signal_safe.SetSigWinchCode(iolib.UNTRAPPED_SIGWINCH) else: # TODO: In process.InitInteractiveShell(), 4 signals are set to # SIG_IGN, not SIG_DFL: @@ -109,7 +109,7 @@ def RemoveUserTrap(self, sig_num): # Should we restore them? It's rare that you type 'trap' in # interactive shells, but it might be more correct. See what other # shells do. - pyos.sigaction(sig_num, SIG_DFL) + iolib.sigaction(sig_num, SIG_DFL) def GetPendingTraps(self): # type: () -> Optional[List[command_t]] diff --git a/core/comp_ui.py b/core/comp_ui.py index 8db9dd0c6..0c668902a 100644 --- a/core/comp_ui.py +++ b/core/comp_ui.py @@ -12,7 +12,7 @@ if TYPE_CHECKING: from frontend.py_readline import Readline from core.util import _DebugFile - from core import pyos + from mycpp import iolib # ANSI escape codes affect the prompt! # https://superuser.com/questions/301353/escape-non-printing-characters-in-a-function-for-a-bash-prompt @@ -316,7 +316,7 @@ def __init__( prompt_state, # type: PromptState debug_f, # type: _DebugFile readline, # type: Optional[Readline] - signal_safe, # type: pyos.SignalSafe + signal_safe, # type: iolib.SignalSafe ): # type: (...) -> None """ diff --git a/core/process.py b/core/process.py index 90e7ec8a6..ef2d2a56a 100644 --- a/core/process.py +++ b/core/process.py @@ -39,6 +39,7 @@ from data_lang import j8_lite from frontend import location from frontend import match +from mycpp import iolib from mycpp import mylib from mycpp.mylib import log, print_stderr, probe, tagswitch, iteritems @@ -109,27 +110,27 @@ def __exit__(self, type, value, traceback): def InitInteractiveShell(signal_safe): - # type: (pyos.SignalSafe) -> None + # type: (iolib.SignalSafe) -> None """Called when initializing an interactive shell.""" # The shell itself should ignore Ctrl-\. - pyos.sigaction(SIGQUIT, SIG_IGN) + iolib.sigaction(SIGQUIT, SIG_IGN) # This prevents Ctrl-Z from suspending OSH in interactive mode. - pyos.sigaction(SIGTSTP, SIG_IGN) + iolib.sigaction(SIGTSTP, SIG_IGN) # More signals from # https://www.gnu.org/software/libc/manual/html_node/Initializing-the-Shell.html # (but not SIGCHLD) - pyos.sigaction(SIGTTOU, SIG_IGN) - pyos.sigaction(SIGTTIN, SIG_IGN) + iolib.sigaction(SIGTTOU, SIG_IGN) + iolib.sigaction(SIGTTIN, SIG_IGN) # Register a callback to receive terminal width changes. # NOTE: In line_input.c, we turned off rl_catch_sigwinch. # This is ALWAYS on, which means that it can cause EINTR, and wait() and # read() have to handle it - pyos.RegisterSignalInterest(SIGWINCH) + iolib.RegisterSignalInterest(SIGWINCH) def SaveFd(fd): @@ -1073,23 +1074,23 @@ def StartProcess(self, why): # shouldn't have this. # https://docs.python.org/2/library/signal.html # See Python/pythonrun.c. - pyos.sigaction(SIGPIPE, SIG_DFL) + iolib.sigaction(SIGPIPE, SIG_DFL) # Respond to Ctrl-\ (core dump) - pyos.sigaction(SIGQUIT, SIG_DFL) + iolib.sigaction(SIGQUIT, SIG_DFL) # Only standalone children should get Ctrl-Z. Pipelines remain in the # foreground because suspending them is difficult with our 'lastpipe' # semantics. pid = posix.getpid() if posix.getpgid(0) == pid and self.parent_pipeline is None: - pyos.sigaction(SIGTSTP, SIG_DFL) + iolib.sigaction(SIGTSTP, SIG_DFL) # More signals from # https://www.gnu.org/software/libc/manual/html_node/Launching-Jobs.html # (but not SIGCHLD) - pyos.sigaction(SIGTTOU, SIG_DFL) - pyos.sigaction(SIGTTIN, SIG_DFL) + iolib.sigaction(SIGTTOU, SIG_DFL) + iolib.sigaction(SIGTTIN, SIG_DFL) self.tracer.OnNewProcess(pid) # clear foreground pipeline for subshells @@ -1861,7 +1862,7 @@ class Waiter(object): """ def __init__(self, job_list, exec_opts, signal_safe, tracer): - # type: (JobList, optview.Exec, pyos.SignalSafe, dev.Tracer) -> None + # type: (JobList, optview.Exec, iolib.SignalSafe, dev.Tracer) -> None self.job_list = job_list self.exec_opts = exec_opts self.signal_safe = signal_safe diff --git a/core/pyos.py b/core/pyos.py index 555fa7960..e1fadbd66 100644 --- a/core/pyos.py +++ b/core/pyos.py @@ -8,12 +8,12 @@ from errno import EINTR import pwd import resource -import signal import select import sys import termios # for read -n import time +from mycpp.iolib import gSignalSafe from mycpp import mops from mycpp.mylib import log @@ -282,164 +282,6 @@ def InputAvailable(fd): return len(r) != 0 -UNTRAPPED_SIGWINCH = -1 - - -class SignalSafe(object): - """State that is shared between the main thread and signal handlers. - - See C++ implementation in cpp/core.h - """ - - def __init__(self): - # type: () -> None - self.pending_signals = [] # type: List[int] - self.last_sig_num = 0 # type: int - self.sigint_trapped = False - self.received_sigint = False - self.received_sigwinch = False - self.sigwinch_code = UNTRAPPED_SIGWINCH - - def UpdateFromSignalHandler(self, sig_num, unused_frame): - # type: (int, Any) -> None - """Receive the given signal, and update shared state. - - This method is registered as a Python signal handler. - """ - self.pending_signals.append(sig_num) - - if sig_num == signal.SIGINT: - self.received_sigint = True - - if sig_num == signal.SIGWINCH: - self.received_sigwinch = True - sig_num = self.sigwinch_code # mutate param - - self.last_sig_num = sig_num - - def LastSignal(self): - # type: () -> int - """Return the number of the last signal received.""" - return self.last_sig_num - - def PollSigInt(self): - # type: () -> bool - """Has SIGINT received since the last time PollSigInt() was called?""" - result = self.received_sigint - self.received_sigint = False - return result - - def PollUntrappedSigInt(self): - # type: () -> bool - """Has SIGINT received since the last time PollSigInt() was called?""" - received = self.PollSigInt() - return received and not self.sigint_trapped - - if 0: - - def SigIntTrapped(self): - # type: () -> bool - return self.sigint_trapped - - def SetSigIntTrapped(self, b): - # type: (bool) -> None - """Set a flag to tell us whether sigint is trapped by the user.""" - self.sigint_trapped = b - - def SetSigWinchCode(self, code): - # type: (int) -> None - """Depending on whether or not SIGWINCH is trapped by a user, it is - expected to report a different code to `wait`. - - SetSigWinchCode() lets us set which code is reported. - """ - self.sigwinch_code = code - - def PollSigWinch(self): - # type: () -> bool - """Has SIGWINCH been received since the last time PollSigWinch() was - called?""" - result = self.received_sigwinch - self.received_sigwinch = False - return result - - def TakePendingSignals(self): - # type: () -> List[int] - """Transfer ownership of queue of pending signals to caller.""" - - # A note on signal-safety here. The main loop might be calling this function - # at the same time a signal is firing and appending to - # `self.pending_signals`. We can forgoe using a lock here - # (which would be problematic for the signal handler) because mutual - # exclusivity should be maintained by the atomic nature of pointer - # assignment (i.e. word-sized writes) on most modern platforms. - # The replacement run list is allocated before the swap, so it can be - # interrupted at any point without consequence. - # This means the signal handler always has exclusive access to - # `self.pending_signals`. In the worst case the signal handler might write to - # `new_queue` and the corresponding trap handler won't get executed - # until the main loop calls this function again. - # NOTE: It's important to distinguish between signal-safety an - # thread-safety here. Signals run in the same process context as the main - # loop, while concurrent threads do not and would have to worry about - # cache-coherence and instruction reordering. - new_queue = [] # type: List[int] - ret = self.pending_signals - self.pending_signals = new_queue - return ret - - def ReuseEmptyList(self, empty_list): - # type: (List[int]) -> None - """This optimization only happens in C++.""" - pass - - -gSignalSafe = None # type: SignalSafe - -gOrigSigIntHandler = None # type: Any - - -def InitSignalSafe(): - # type: () -> SignalSafe - """Set global instance so the signal handler can access it.""" - global gSignalSafe - gSignalSafe = SignalSafe() - - # See - # - demo/cpython/keyboard_interrupt.py - # - pyos::InitSignalSafe() - - # In C++, we do - # RegisterSignalInterest(signal.SIGINT) - - global gOrigSigIntHandler - gOrigSigIntHandler = signal.signal(signal.SIGINT, - gSignalSafe.UpdateFromSignalHandler) - - return gSignalSafe - - -def sigaction(sig_num, handler): - # type: (int, Any) -> None - """ - Handle a signal with SIG_DFL or SIG_IGN, not our own signal handler. - """ - - # SIGINT and SIGWINCH must be registered through SignalSafe - assert sig_num != signal.SIGINT - assert sig_num != signal.SIGWINCH - signal.signal(sig_num, handler) - - -def RegisterSignalInterest(sig_num): - # type: (int) -> None - """Have the kernel notify the main loop about the given signal.""" - #log('RegisterSignalInterest %d', sig_num) - - assert gSignalSafe is not None - signal.signal(sig_num, gSignalSafe.UpdateFromSignalHandler) - - def MakeDirCacheKey(path): # type: (str) -> Tuple[str, int] """Returns a pair (path with last modified time) that can be used to cache diff --git a/core/shell.py b/core/shell.py index 65478a2eb..364310726 100644 --- a/core/shell.py +++ b/core/shell.py @@ -20,7 +20,6 @@ from core import completion from core import main_loop from core import optview -from core import pyos from core import process from core import pyutil from core import state @@ -78,6 +77,7 @@ from osh import split from osh import word_eval +from mycpp import iolib from mycpp import mops from mycpp import mylib from mycpp.mylib import NewDict, iteritems, print_stderr, log @@ -477,10 +477,7 @@ def Main( multi_trace) fd_state.tracer = tracer # circular dep - # RegisterSignalInterest should return old sigint handler - # then InteractiveLineReader can use it - # InteractiveLineReader - signal_safe = pyos.InitSignalSafe() + signal_safe = iolib.InitSignalSafe() trap_state = trap_osh.TrapState(signal_safe) waiter = process.Waiter(job_list, exec_opts, signal_safe, tracer) diff --git a/cpp/core.cc b/cpp/core.cc index fc846b954..e34fcbb7c 100644 --- a/cpp/core.cc +++ b/cpp/core.cc @@ -23,18 +23,17 @@ #include "_gen/cpp/build_stamp.h" // gCommitHash #include "_gen/frontend/consts.h" // gVersion #include "cpp/embedded_file.h" +#include "mycpp/gc_iolib.h" extern char** environ; namespace pyos { -SignalSafe* gSignalSafe = nullptr; - Tuple2 WaitPid(int waitpid_options) { int status; int result = ::waitpid(-1, &status, WUNTRACED | waitpid_options); if (result < 0) { - if (errno == EINTR && gSignalSafe->PollUntrappedSigInt()) { + if (errno == EINTR && iolib::gSignalSafe->PollUntrappedSigInt()) { throw Alloc(); } return Tuple2(-1, errno); @@ -47,7 +46,7 @@ Tuple2 Read(int fd, int n, List* chunks) { int length = ::read(fd, s->data(), n); if (length < 0) { - if (errno == EINTR && gSignalSafe->PollUntrappedSigInt()) { + if (errno == EINTR && iolib::gSignalSafe->PollUntrappedSigInt()) { throw Alloc(); } return Tuple2(-1, errno); @@ -67,7 +66,7 @@ Tuple2 ReadByte(int fd) { unsigned char buf[1]; ssize_t n = read(fd, &buf, 1); if (n < 0) { // read error - if (errno == EINTR && gSignalSafe->PollUntrappedSigInt()) { + if (errno == EINTR && iolib::gSignalSafe->PollUntrappedSigInt()) { throw Alloc(); } return Tuple2(-1, errno); @@ -263,43 +262,6 @@ IOError_OSError* FlushStdout() { return nullptr; } -SignalSafe* InitSignalSafe() { - gSignalSafe = Alloc(); - gHeap.RootGlobalVar(gSignalSafe); - - RegisterSignalInterest(SIGINT); // for KeyboardInterrupt checks - - return gSignalSafe; -} - -// Note that the Python implementation of pyos.sigaction() calls -// signal.signal(), which calls PyOS_setsig(), which calls sigaction() #ifdef -// HAVE_SIGACTION. -void sigaction(int sig_num, void (*handler)(int)) { - // SIGINT and SIGWINCH must be registered through SignalSafe - DCHECK(sig_num != SIGINT); - DCHECK(sig_num != SIGWINCH); - - struct sigaction act = {}; - act.sa_handler = handler; - if (sigaction(sig_num, &act, nullptr) != 0) { - throw Alloc(errno); - } -} - -static void OurSignalHandler(int sig_num) { - assert(gSignalSafe != nullptr); - gSignalSafe->UpdateFromSignalHandler(sig_num); -} - -void RegisterSignalInterest(int sig_num) { - struct sigaction act = {}; - act.sa_handler = OurSignalHandler; - if (sigaction(sig_num, &act, nullptr) != 0) { - throw Alloc(errno); - } -} - Tuple2* MakeDirCacheKey(BigStr* path) { struct stat st; if (::stat(path->data(), &st) == -1) { diff --git a/cpp/core.h b/cpp/core.h index 7e1576efc..f6d106b25 100644 --- a/cpp/core.h +++ b/cpp/core.h @@ -4,20 +4,8 @@ #define CORE_H #include // passwd -#include #include -// For now, we assume that simple int and pointer operations are atomic, rather -// than using std::atomic. Could be a ./configure option later. -// -// See doc/portability.md. - -#define LOCK_FREE_ATOMICS 0 - -#if LOCK_FREE_ATOMICS - #include -#endif - #include "_gen/frontend/syntax.asdl.h" #include "cpp/pgen2.h" #include "mycpp/runtime.h" @@ -33,7 +21,6 @@ const int TERM_ICANON = ICANON; const int TERM_ECHO = ECHO; const int EOF_SENTINEL = 256; const int NEWLINE_CH = 10; -const int UNTRAPPED_SIGWINCH = -1; Tuple2 WaitPid(int waitpid_options); Tuple2 Read(int fd, int n, List* chunks); @@ -98,155 +85,6 @@ IOError_OSError* FlushStdout(); Tuple2 PushTermAttrs(int fd, int mask); void PopTermAttrs(int fd, int orig_local_modes, void* term_attrs); -// Make the signal queue slab 4096 bytes, including the GC header. See -// cpp/core_test.cc. -const int kMaxPendingSignals = 1022; - -class SignalSafe { - // State that is shared between the main thread and signal handlers. - public: - SignalSafe() - : pending_signals_(AllocSignalList()), - empty_list_(AllocSignalList()), // to avoid repeated allocation - last_sig_num_(0), - received_sigint_(false), - received_sigwinch_(false), - sigwinch_code_(UNTRAPPED_SIGWINCH), - num_dropped_(0) { - } - - // Called from signal handling context. Do not allocate. - void UpdateFromSignalHandler(int sig_num) { - if (pending_signals_->len_ < pending_signals_->capacity_) { - // We can append without allocating - pending_signals_->append(sig_num); - } else { - // Unlikely: we would have to allocate. Just increment a counter, which - // we could expose somewhere in the UI. - num_dropped_++; - } - - if (sig_num == SIGINT) { - received_sigint_ = true; - } - - if (sig_num == SIGWINCH) { - received_sigwinch_ = true; - sig_num = sigwinch_code_; // mutate param - } - -#if LOCK_FREE_ATOMICS - last_sig_num_.store(sig_num); -#else - last_sig_num_ = sig_num; -#endif - } - - // Main thread takes signals so it can run traps. - List* TakePendingSignals() { - List* ret = pending_signals_; - - // Make sure we have a distinct list to reuse. - DCHECK(empty_list_ != pending_signals_); - pending_signals_ = empty_list_; - - return ret; - } - - // Main thread returns the same list as an optimization to avoid allocation. - void ReuseEmptyList(List* empty_list) { - DCHECK(empty_list != pending_signals_); // must be different - DCHECK(len(empty_list) == 0); // main thread clears - DCHECK(empty_list->capacity_ == kMaxPendingSignals); - - empty_list_ = empty_list; - } - - // Main thread wants to get the last signal received. - int LastSignal() { -#if LOCK_FREE_ATOMICS - return last_sig_num_.load(); -#else - return last_sig_num_; -#endif - } - - void SetSigIntTrapped(bool b) { - sigint_trapped_ = b; - } - - // Used by pyos.WaitPid, Read, ReadByte. - bool PollSigInt() { - bool result = received_sigint_; - received_sigint_ = false; - return result; - } - - // Used by osh/cmd_eval.py. Main loop wants to know if SIGINT was received - // since the last time PollSigInt was called. - bool PollUntrappedSigInt() { - bool received = PollSigInt(); // clears a flag - return received && !sigint_trapped_; - } - - // Main thread tells us whether SIGWINCH is trapped. - void SetSigWinchCode(int code) { - sigwinch_code_ = code; - } - - // Main thread wants to know if SIGWINCH was received since the last time - // PollSigWinch was called. - bool PollSigWinch() { - bool result = received_sigwinch_; - received_sigwinch_ = false; - return result; - } - - static constexpr uint32_t field_mask() { - return maskbit(offsetof(SignalSafe, pending_signals_)) | - maskbit(offsetof(SignalSafe, empty_list_)); - } - - static constexpr ObjHeader obj_header() { - return ObjHeader::ClassFixed(field_mask(), sizeof(SignalSafe)); - } - - List* pending_signals_; // public for testing - List* empty_list_; - - private: - // Enforce private state because two different "threads" will use it! - - // Reserve a fixed number of signals. - List* AllocSignalList() { - List* ret = NewList(); - ret->reserve(kMaxPendingSignals); - return ret; - } - -#if LOCK_FREE_ATOMICS - std::atomic last_sig_num_; -#else - int last_sig_num_; -#endif - // Not sufficient: volatile sig_atomic_t last_sig_num_; - - bool sigint_trapped_; - int received_sigint_; - int received_sigwinch_; - int sigwinch_code_; - int num_dropped_; -}; - -extern SignalSafe* gSignalSafe; - -// Allocate global and return it. -SignalSafe* InitSignalSafe(); - -void sigaction(int sig_num, void (*handler)(int)); - -void RegisterSignalInterest(int sig_num); - Tuple2* MakeDirCacheKey(BigStr* path); } // namespace pyos diff --git a/cpp/core_test.cc b/cpp/core_test.cc index 602ef976e..505d38abb 100644 --- a/cpp/core_test.cc +++ b/cpp/core_test.cc @@ -11,6 +11,7 @@ #include "cpp/embedded_file.h" #include "cpp/stdlib.h" // posix::getcwd #include "mycpp/gc_builtins.h" // IOError_OSError +#include "mycpp/gc_iolib.h" // iolib #include "vendor/greatest.h" TEST for_test_coverage() { @@ -235,92 +236,6 @@ TEST strerror_test() { PASS(); } -TEST signal_test() { - pyos::SignalSafe* signal_safe = pyos::InitSignalSafe(); - - { - List* q = signal_safe->TakePendingSignals(); - ASSERT(q != nullptr); - ASSERT_EQ(0, len(q)); - signal_safe->ReuseEmptyList(q); - } - - pid_t mypid = getpid(); - - pyos::RegisterSignalInterest(SIGUSR1); - pyos::RegisterSignalInterest(SIGUSR2); - - kill(mypid, SIGUSR1); - ASSERT_EQ(SIGUSR1, signal_safe->LastSignal()); - - kill(mypid, SIGUSR2); - ASSERT_EQ(SIGUSR2, signal_safe->LastSignal()); - - { - List* q = signal_safe->TakePendingSignals(); - ASSERT(q != nullptr); - ASSERT_EQ(2, len(q)); - ASSERT_EQ(SIGUSR1, q->at(0)); - ASSERT_EQ(SIGUSR2, q->at(1)); - - q->clear(); - signal_safe->ReuseEmptyList(q); - } - - pyos::sigaction(SIGUSR1, SIG_IGN); - kill(mypid, SIGUSR1); - { - List* q = signal_safe->TakePendingSignals(); - ASSERT(q != nullptr); - ASSERT(len(q) == 0); - signal_safe->ReuseEmptyList(q); - } - pyos::sigaction(SIGUSR2, SIG_IGN); - - pyos::RegisterSignalInterest(SIGWINCH); - - kill(mypid, SIGWINCH); - ASSERT_EQ(pyos::UNTRAPPED_SIGWINCH, signal_safe->LastSignal()); - - signal_safe->SetSigWinchCode(SIGWINCH); - - kill(mypid, SIGWINCH); - ASSERT_EQ(SIGWINCH, signal_safe->LastSignal()); - { - List* q = signal_safe->TakePendingSignals(); - ASSERT(q != nullptr); - ASSERT_EQ(2, len(q)); - ASSERT_EQ(SIGWINCH, q->at(0)); - ASSERT_EQ(SIGWINCH, q->at(1)); - } - - PASS(); -} - -TEST signal_safe_test() { - pyos::SignalSafe signal_safe; - - List* received = signal_safe.TakePendingSignals(); - - // We got now signals - ASSERT_EQ_FMT(0, len(received), "%d"); - - // The existing queue is of length 0 - ASSERT_EQ_FMT(0, len(signal_safe.pending_signals_), "%d"); - - // Capacity is a ROUND NUMBER from the allocator's POV - // There's no convenient way to test the obj_len we pass to gHeap.Allocate, - // but it should be (1022 + 2) * 4. - ASSERT_EQ_FMT(1022, signal_safe.pending_signals_->capacity_, "%d"); - - // Register too many signals - for (int i = 0; i < pyos::kMaxPendingSignals + 10; ++i) { - signal_safe.UpdateFromSignalHandler(SIGINT); - } - - PASS(); -} - TEST passwd_test() { uid_t my_uid = getuid(); BigStr* username = pyos::GetUserName(my_uid); @@ -387,8 +302,8 @@ TEST asan_global_leak_test() { // manual demo TEST waitpid_demo() { - pyos::InitSignalSafe(); - pyos::RegisterSignalInterest(SIGINT); + iolib::InitSignalSafe(); + iolib::RegisterSignalInterest(SIGINT); int result = fork(); if (result < 0) { @@ -433,9 +348,6 @@ int main(int argc, char** argv) { RUN_TEST(pyutil_test); RUN_TEST(strerror_test); - RUN_TEST(signal_test); - RUN_TEST(signal_safe_test); - RUN_TEST(passwd_test); RUN_TEST(dir_cache_key_test); RUN_TEST(asan_global_leak_test); diff --git a/cpp/frontend_pyreadline.cc b/cpp/frontend_pyreadline.cc index 98239ef97..2b5f2f3cc 100644 --- a/cpp/frontend_pyreadline.cc +++ b/cpp/frontend_pyreadline.cc @@ -14,6 +14,7 @@ #endif #include "cpp/core.h" +#include "mycpp/gc_mylib.h" namespace py_readline { @@ -117,7 +118,7 @@ BigStr* readline(BigStr* prompt) { FD_SET(fileno(rl_instream), &fds); int ec = select(FD_SETSIZE, &fds, NULL, NULL, NULL); if (ec == -1) { - if (errno == EINTR && pyos::gSignalSafe->PollSigInt()) { + if (errno == EINTR && iolib::gSignalSafe->PollSigInt()) { // User is trying to cancel. Abort and cleanup readline state. rl_free_line_state(); rl_callback_sigcleanup(); diff --git a/frontend/reader.py b/frontend/reader.py index a03c0d729..86f4e0d28 100644 --- a/frontend/reader.py +++ b/frontend/reader.py @@ -11,6 +11,7 @@ from _devbuild.gen.id_kind_asdl import Id from core.error import p_die +from mycpp import iolib from mycpp import mylib from mycpp.mylib import log @@ -164,7 +165,11 @@ def _PlainPromptInput(prompt): Returns line WITH trailing newline, like Python's f.readline(), and unlike raw_input() / GNU readline - Same interface as readline.prompt_input(). + Same interface as readline.prompt_input(): + + Raises + EOFError: on Ctrl-D + KeyboardInterrupt: on Ctrl-C """ w = mylib.Stderr() w.write(prompt) @@ -221,9 +226,8 @@ def _ReadlinePromptInput(self): # A cleaner way to do this would be to fork CPython's raw_input() # so it handles EINTR. It's called in frontend/pyreadline.py import signal - from core import pyos - tmp = signal.signal(signal.SIGINT, pyos.gOrigSigIntHandler) + tmp = signal.signal(signal.SIGINT, iolib.gOrigSigIntHandler) try: line = self.line_input.prompt_input(self.prompt_str) finally: diff --git a/mycpp/NINJA_subgraph.py b/mycpp/NINJA_subgraph.py index f2455c654..18d0a9ecd 100644 --- a/mycpp/NINJA_subgraph.py +++ b/mycpp/NINJA_subgraph.py @@ -24,6 +24,7 @@ def DefineTargets(ru): srcs=[ 'mycpp/bump_leak_heap.cc', 'mycpp/gc_builtins.cc', + 'mycpp/gc_iolib.cc', 'mycpp/gc_mops.cc', 'mycpp/gc_mylib.cc', 'mycpp/gc_str.cc', @@ -47,6 +48,7 @@ def DefineTargets(ru): 'mycpp/gc_heap_test.cc', 'mycpp/gc_stress_test.cc', 'mycpp/gc_builtins_test.cc', + 'mycpp/gc_iolib_test.cc', 'mycpp/gc_mops_test.cc', 'mycpp/gc_mylib_test.cc', 'mycpp/gc_dict_test.cc', @@ -211,7 +213,7 @@ def TranslatorSubgraph(ru, translator, ex): example_matrix = [ ('cxx', 'opt'), # for benchmarks ('cxx', 'opt-sh'), # for benchmarks - ('cxx', 'asan'), # need this for running the examples in CI + ('cxx', 'asan'), # need this for running the examples in CI ('cxx', 'asan+gcalways'), ] else: @@ -250,9 +252,10 @@ def NinjaGraph(ru): n.newline() # mycpp and pea have the same interface - n.rule('translate-mycpp', - command='_bin/shwrap/mycpp_main $mypypath $out $in $extra_mycpp_opts', - description='mycpp $mypypath $out $in') + n.rule( + 'translate-mycpp', + command='_bin/shwrap/mycpp_main $mypypath $out $in $extra_mycpp_opts', + description='mycpp $mypypath $out $in') n.newline() n.rule('translate-pea', diff --git a/mycpp/gc_iolib.cc b/mycpp/gc_iolib.cc new file mode 100644 index 000000000..3c1ae966a --- /dev/null +++ b/mycpp/gc_iolib.cc @@ -0,0 +1,46 @@ +#include "mycpp/gc_iolib.h" + +#include + +namespace iolib { + +SignalSafe* gSignalSafe = nullptr; + +SignalSafe* InitSignalSafe() { + gSignalSafe = Alloc(); + gHeap.RootGlobalVar(gSignalSafe); + + RegisterSignalInterest(SIGINT); // for KeyboardInterrupt checks + + return gSignalSafe; +} + +static void OurSignalHandler(int sig_num) { + assert(gSignalSafe != nullptr); + gSignalSafe->UpdateFromSignalHandler(sig_num); +} + +void RegisterSignalInterest(int sig_num) { + struct sigaction act = {}; + act.sa_handler = OurSignalHandler; + if (sigaction(sig_num, &act, nullptr) != 0) { + throw Alloc(errno); + } +} + +// Note that the Python implementation of pyos.sigaction() calls +// signal.signal(), which calls PyOS_setsig(), which calls sigaction() #ifdef +// HAVE_SIGACTION. +void sigaction(int sig_num, void (*handler)(int)) { + // SIGINT and SIGWINCH must be registered through SignalSafe + DCHECK(sig_num != SIGINT); + DCHECK(sig_num != SIGWINCH); + + struct sigaction act = {}; + act.sa_handler = handler; + if (sigaction(sig_num, &act, nullptr) != 0) { + throw Alloc(errno); + } +} + +} // namespace iolib diff --git a/mycpp/gc_iolib.h b/mycpp/gc_iolib.h new file mode 100644 index 000000000..adffd4a05 --- /dev/null +++ b/mycpp/gc_iolib.h @@ -0,0 +1,175 @@ +// gc_iolib.h - corresponds to mycpp/iolib.py + +#ifndef MYCPP_GC_IOLIB_H +#define MYCPP_GC_IOLIB_H + +// For now, we assume that simple int and pointer operations are atomic, rather +// than using std::atomic. Could be a ./configure option later. +// +// See doc/portability.md. + +#define LOCK_FREE_ATOMICS 0 + +#if LOCK_FREE_ATOMICS + #include +#endif +#include + +#include "mycpp/gc_list.h" + +namespace iolib { + +const int UNTRAPPED_SIGWINCH = -1; + +// Make the signal queue slab 4096 bytes, including the GC header. See +// cpp/core_test.cc. +const int kMaxPendingSignals = 1022; + +class SignalSafe { + // State that is shared between the main thread and signal handlers. + public: + SignalSafe() + : pending_signals_(AllocSignalList()), + empty_list_(AllocSignalList()), // to avoid repeated allocation + last_sig_num_(0), + received_sigint_(false), + received_sigwinch_(false), + sigwinch_code_(UNTRAPPED_SIGWINCH), + num_dropped_(0) { + } + + // Called from signal handling context. Do not allocate. + void UpdateFromSignalHandler(int sig_num) { + if (pending_signals_->len_ < pending_signals_->capacity_) { + // We can append without allocating + pending_signals_->append(sig_num); + } else { + // Unlikely: we would have to allocate. Just increment a counter, which + // we could expose somewhere in the UI. + num_dropped_++; + } + + if (sig_num == SIGINT) { + received_sigint_ = true; + } + + if (sig_num == SIGWINCH) { + received_sigwinch_ = true; + sig_num = sigwinch_code_; // mutate param + } + +#if LOCK_FREE_ATOMICS + last_sig_num_.store(sig_num); +#else + last_sig_num_ = sig_num; +#endif + } + + // Main thread takes signals so it can run traps. + List* TakePendingSignals() { + List* ret = pending_signals_; + + // Make sure we have a distinct list to reuse. + DCHECK(empty_list_ != pending_signals_); + pending_signals_ = empty_list_; + + return ret; + } + + // Main thread returns the same list as an optimization to avoid allocation. + void ReuseEmptyList(List* empty_list) { + DCHECK(empty_list != pending_signals_); // must be different + DCHECK(len(empty_list) == 0); // main thread clears + DCHECK(empty_list->capacity_ == kMaxPendingSignals); + + empty_list_ = empty_list; + } + + // Main thread wants to get the last signal received. + int LastSignal() { +#if LOCK_FREE_ATOMICS + return last_sig_num_.load(); +#else + return last_sig_num_; +#endif + } + + void SetSigIntTrapped(bool b) { + sigint_trapped_ = b; + } + + // Used by pyos.WaitPid, Read, ReadByte. + bool PollSigInt() { + bool result = received_sigint_; + received_sigint_ = false; + return result; + } + + // Used by osh/cmd_eval.py. Main loop wants to know if SIGINT was received + // since the last time PollSigInt was called. + bool PollUntrappedSigInt() { + bool received = PollSigInt(); // clears a flag + return received && !sigint_trapped_; + } + + // Main thread tells us whether SIGWINCH is trapped. + void SetSigWinchCode(int code) { + sigwinch_code_ = code; + } + + // Main thread wants to know if SIGWINCH was received since the last time + // PollSigWinch was called. + bool PollSigWinch() { + bool result = received_sigwinch_; + received_sigwinch_ = false; + return result; + } + + static constexpr uint32_t field_mask() { + return maskbit(offsetof(SignalSafe, pending_signals_)) | + maskbit(offsetof(SignalSafe, empty_list_)); + } + + static constexpr ObjHeader obj_header() { + return ObjHeader::ClassFixed(field_mask(), sizeof(SignalSafe)); + } + + List* pending_signals_; // public for testing + List* empty_list_; + + private: + // Enforce private state because two different "threads" will use it! + + // Reserve a fixed number of signals. + List* AllocSignalList() { + List* ret = NewList(); + ret->reserve(kMaxPendingSignals); + return ret; + } + +#if LOCK_FREE_ATOMICS + std::atomic last_sig_num_; +#else + int last_sig_num_; +#endif + // Not sufficient: volatile sig_atomic_t last_sig_num_; + + bool sigint_trapped_; + int received_sigint_; + int received_sigwinch_; + int sigwinch_code_; + int num_dropped_; +}; + +extern SignalSafe* gSignalSafe; + +// Allocate global and return it. +SignalSafe* InitSignalSafe(); + +void RegisterSignalInterest(int sig_num); + +void sigaction(int sig_num, void (*handler)(int)); + +} // namespace iolib + +#endif // MYCPP_GC_IOLIB_H diff --git a/mycpp/gc_iolib_test.cc b/mycpp/gc_iolib_test.cc new file mode 100644 index 000000000..008e22f3a --- /dev/null +++ b/mycpp/gc_iolib_test.cc @@ -0,0 +1,109 @@ +#include "mycpp/gc_iolib.h" + +#include + +#include "mycpp/gc_alloc.h" // gHeap +#include "vendor/greatest.h" + +TEST signal_test() { + iolib::SignalSafe* signal_safe = iolib::InitSignalSafe(); + + { + List* q = signal_safe->TakePendingSignals(); + ASSERT(q != nullptr); + ASSERT_EQ(0, len(q)); + signal_safe->ReuseEmptyList(q); + } + + pid_t mypid = getpid(); + + iolib::RegisterSignalInterest(SIGUSR1); + iolib::RegisterSignalInterest(SIGUSR2); + + kill(mypid, SIGUSR1); + ASSERT_EQ(SIGUSR1, signal_safe->LastSignal()); + + kill(mypid, SIGUSR2); + ASSERT_EQ(SIGUSR2, signal_safe->LastSignal()); + + { + List* q = signal_safe->TakePendingSignals(); + ASSERT(q != nullptr); + ASSERT_EQ(2, len(q)); + ASSERT_EQ(SIGUSR1, q->at(0)); + ASSERT_EQ(SIGUSR2, q->at(1)); + + q->clear(); + signal_safe->ReuseEmptyList(q); + } + + iolib::sigaction(SIGUSR1, SIG_IGN); + kill(mypid, SIGUSR1); + { + List* q = signal_safe->TakePendingSignals(); + ASSERT(q != nullptr); + ASSERT(len(q) == 0); + signal_safe->ReuseEmptyList(q); + } + iolib::sigaction(SIGUSR2, SIG_IGN); + + iolib::RegisterSignalInterest(SIGWINCH); + + kill(mypid, SIGWINCH); + ASSERT_EQ(iolib::UNTRAPPED_SIGWINCH, signal_safe->LastSignal()); + + signal_safe->SetSigWinchCode(SIGWINCH); + + kill(mypid, SIGWINCH); + ASSERT_EQ(SIGWINCH, signal_safe->LastSignal()); + { + List* q = signal_safe->TakePendingSignals(); + ASSERT(q != nullptr); + ASSERT_EQ(2, len(q)); + ASSERT_EQ(SIGWINCH, q->at(0)); + ASSERT_EQ(SIGWINCH, q->at(1)); + } + + PASS(); +} + +TEST signal_safe_test() { + iolib::SignalSafe signal_safe; + + List* received = signal_safe.TakePendingSignals(); + + // We got now signals + ASSERT_EQ_FMT(0, len(received), "%d"); + + // The existing queue is of length 0 + ASSERT_EQ_FMT(0, len(signal_safe.pending_signals_), "%d"); + + // Capacity is a ROUND NUMBER from the allocator's POV + // There's no convenient way to test the obj_len we pass to gHeap.Allocate, + // but it should be (1022 + 2) * 4. + ASSERT_EQ_FMT(1022, signal_safe.pending_signals_->capacity_, "%d"); + + // Register too many signals + for (int i = 0; i < iolib::kMaxPendingSignals + 10; ++i) { + signal_safe.UpdateFromSignalHandler(SIGINT); + } + + PASS(); +} + +GREATEST_MAIN_DEFS(); + +int main(int argc, char** argv) { + gHeap.Init(); + + GREATEST_MAIN_BEGIN(); + + RUN_TEST(signal_test); + RUN_TEST(signal_safe_test); + + gHeap.CleanProcessExit(); + + GREATEST_MAIN_END(); /* display results */ + + return 0; +} diff --git a/mycpp/gc_mylib.cc b/mycpp/gc_mylib.cc index dfa90d2f0..37a6ad2a4 100644 --- a/mycpp/gc_mylib.cc +++ b/mycpp/gc_mylib.cc @@ -114,6 +114,15 @@ BigStr* CFile::readline() { // man page says the buffer should be freed even if getline fails free(line); +#if 0 + // Need to raise KeyboardInterrupt like mylib.Stdin().readline() does in + // Python! This affects _PlainPromptInput() in frontend/reader.py + // gSignalSafe. But the dependency on gSignalSafe is "inverted". + if (errno == EINTR && gSignalSafe->PollUntrappedSigInt()) { + throw Alloc(); + } +#endif + if (errno != 0) { // Unexpected error // log("getline() error: %s", strerror(errno)); throw Alloc(errno); diff --git a/mycpp/iolib.py b/mycpp/iolib.py new file mode 100644 index 000000000..a20d67be1 --- /dev/null +++ b/mycpp/iolib.py @@ -0,0 +1,165 @@ +""" +mylib.py: Python stubs/interfaces that are reimplemented in C++, not directly +translated. +""" +from __future__ import print_function + +import signal + +from typing import List, Any + +UNTRAPPED_SIGWINCH = -1 + + +class SignalSafe(object): + """State that is shared between the main thread and signal handlers. + + See C++ implementation in cpp/core.h + """ + + def __init__(self): + # type: () -> None + self.pending_signals = [] # type: List[int] + self.last_sig_num = 0 # type: int + self.sigint_trapped = False + self.received_sigint = False + self.received_sigwinch = False + self.sigwinch_code = UNTRAPPED_SIGWINCH + + def UpdateFromSignalHandler(self, sig_num, unused_frame): + # type: (int, Any) -> None + """Receive the given signal, and update shared state. + + This method is registered as a Python signal handler. + """ + self.pending_signals.append(sig_num) + + if sig_num == signal.SIGINT: + self.received_sigint = True + + if sig_num == signal.SIGWINCH: + self.received_sigwinch = True + sig_num = self.sigwinch_code # mutate param + + self.last_sig_num = sig_num + + def LastSignal(self): + # type: () -> int + """Return the number of the last signal received.""" + return self.last_sig_num + + def PollSigInt(self): + # type: () -> bool + """Has SIGINT received since the last time PollSigInt() was called?""" + result = self.received_sigint + self.received_sigint = False + return result + + def PollUntrappedSigInt(self): + # type: () -> bool + """Has SIGINT received since the last time PollSigInt() was called?""" + received = self.PollSigInt() + return received and not self.sigint_trapped + + if 0: + + def SigIntTrapped(self): + # type: () -> bool + return self.sigint_trapped + + def SetSigIntTrapped(self, b): + # type: (bool) -> None + """Set a flag to tell us whether sigint is trapped by the user.""" + self.sigint_trapped = b + + def SetSigWinchCode(self, code): + # type: (int) -> None + """Depending on whether or not SIGWINCH is trapped by a user, it is + expected to report a different code to `wait`. + + SetSigWinchCode() lets us set which code is reported. + """ + self.sigwinch_code = code + + def PollSigWinch(self): + # type: () -> bool + """Has SIGWINCH been received since the last time PollSigWinch() was + called?""" + result = self.received_sigwinch + self.received_sigwinch = False + return result + + def TakePendingSignals(self): + # type: () -> List[int] + """Transfer ownership of queue of pending signals to caller.""" + + # A note on signal-safety here. The main loop might be calling this function + # at the same time a signal is firing and appending to + # `self.pending_signals`. We can forgoe using a lock here + # (which would be problematic for the signal handler) because mutual + # exclusivity should be maintained by the atomic nature of pointer + # assignment (i.e. word-sized writes) on most modern platforms. + # The replacement run list is allocated before the swap, so it can be + # interrupted at any point without consequence. + # This means the signal handler always has exclusive access to + # `self.pending_signals`. In the worst case the signal handler might write to + # `new_queue` and the corresponding trap handler won't get executed + # until the main loop calls this function again. + # NOTE: It's important to distinguish between signal-safety an + # thread-safety here. Signals run in the same process context as the main + # loop, while concurrent threads do not and would have to worry about + # cache-coherence and instruction reordering. + new_queue = [] # type: List[int] + ret = self.pending_signals + self.pending_signals = new_queue + return ret + + def ReuseEmptyList(self, empty_list): + # type: (List[int]) -> None + """This optimization only happens in C++.""" + pass + + +gSignalSafe = None # type: SignalSafe + +gOrigSigIntHandler = None # type: Any + + +def InitSignalSafe(): + # type: () -> SignalSafe + """Set global instance so the signal handler can access it.""" + global gSignalSafe + gSignalSafe = SignalSafe() + + # See + # - demo/cpython/keyboard_interrupt.py + # - pyos::InitSignalSafe() + + # In C++, we do + # RegisterSignalInterest(signal.SIGINT) + + global gOrigSigIntHandler + gOrigSigIntHandler = signal.signal(signal.SIGINT, + gSignalSafe.UpdateFromSignalHandler) + + return gSignalSafe + + +def RegisterSignalInterest(sig_num): + # type: (int) -> None + """Have the kernel notify the main loop about the given signal.""" + #log('RegisterSignalInterest %d', sig_num) + + assert gSignalSafe is not None + signal.signal(sig_num, gSignalSafe.UpdateFromSignalHandler) + + +def sigaction(sig_num, handler): + # type: (int, Any) -> None + """ + Handle a signal with SIG_DFL or SIG_IGN, not our own signal handler. + """ + # SIGINT and SIGWINCH must be registered through SignalSafe + assert sig_num != signal.SIGINT + assert sig_num != signal.SIGWINCH + signal.signal(sig_num, handler) diff --git a/mycpp/mylib.py b/mycpp/mylib.py index 3782ee9ee..59b274436 100644 --- a/mycpp/mylib.py +++ b/mycpp/mylib.py @@ -1,5 +1,6 @@ """ -mylib.py +mylib.py: Python stubs/interfaces that are reimplemented in C++, not directly +translated. """ from __future__ import print_function diff --git a/mycpp/runtime.h b/mycpp/runtime.h index 5c2ccce95..8ed87fa5b 100644 --- a/mycpp/runtime.h +++ b/mycpp/runtime.h @@ -13,6 +13,7 @@ #include "mycpp/gc_list.h" #include "mycpp/gc_dict.h" +#include "mycpp/gc_iolib.h" #include "mycpp/gc_mops.h" // math ops #include "mycpp/gc_mylib.h" // Python-like file I/O, etc. #include "mycpp/hash.h" diff --git a/osh/cmd_eval.py b/osh/cmd_eval.py index f43df96aa..e9b20e4e6 100644 --- a/osh/cmd_eval.py +++ b/osh/cmd_eval.py @@ -85,6 +85,7 @@ from osh import braces from osh import sh_expr_eval from osh import word_eval +from mycpp import iolib from mycpp import mops from mycpp import mylib from mycpp.mylib import log, probe, switch, tagswitch @@ -271,7 +272,7 @@ def __init__( arena, # type: Arena cmd_deps, # type: Deps trap_state, # type: trap_osh.TrapState - signal_safe, # type: pyos.SignalSafe + signal_safe, # type: iolib.SignalSafe ): # type: (...) -> None """ diff --git a/pea/oils-typecheck.txt b/pea/oils-typecheck.txt index fd5e620b9..db913468e 100644 --- a/pea/oils-typecheck.txt +++ b/pea/oils-typecheck.txt @@ -33,6 +33,7 @@ builtin/method_io.py builtin/method_list.py builtin/method_other.py builtin/method_str.py +builtin/method_type.py builtin/misc_osh.py builtin/module_ysh.py builtin/printf_osh.py @@ -82,6 +83,7 @@ frontend/py_readline.py frontend/reader.py frontend/signal_def.py frontend/typed_args.py +mycpp/iolib.py mycpp/mops.py osh/arith_parse.py osh/bool_parse.py diff --git a/prebuilt/dynamic-deps/filter-translate.txt b/prebuilt/dynamic-deps/filter-translate.txt index 648dcfff2..1b954c581 100644 --- a/prebuilt/dynamic-deps/filter-translate.txt +++ b/prebuilt/dynamic-deps/filter-translate.txt @@ -9,6 +9,7 @@ data_lang/py.* frontend/py.*\.py frontend/consts.py frontend/match.py +mycpp/iolib.py mycpp/mops.py pgen2/grammar.py pgen2/pnode.py