comparison libinterp/corefcn/sighandlers.cc @ 31633:d9970470108a stable

Remove several race conditions with signal handler (bug #61370). This patch removes several race conditions between the interpreter and the Ctrl-C signal handler, as described in more detail in bug #61370. * libinterp/corefcn/sighandlers.h: Make can_interrupt atomic. * libinterp/corefcn/sighandlers.cc: Make can_interrupt, signals_caught atomic, use atomic value read for signals_caught, octave_interrupt_state, change 1 to true. * liboctave/util/quit.h: Move octave_interrupt_state and octave_signal_caught to C++ only code, rewrite octave_quit() function, rewrite OCTAVE_QUIT macro to use octave_quit_c() function. * liboctave/util/quit.cc: Make octave_interrupt_state and octave_signal_caught atomic, add new wrapper function octave_quit_c(). * libinterp/corefcn/interpreter.h: Make octave_initialized atomic. * libinterp/corefcn/interpreter.cc: Make octave_initialized atomic, change 0 to false. * liboctave/util/action-container.h: New namespace "util", make m_val atomic. * liboctave/util/oct-atomic.c: Reorder preprocessor if-elif-else ladder. * liboctave/util/cmd-edit.cc: Change 0 to false.
author Reinhard Resch <r_resch@a1.net>
date Sat, 03 Dec 2022 10:36:59 -0500
parents aac27ad79be6
children 7d3467f8d713
comparison
equal deleted inserted replaced
31631:ec1f34091635 31633:d9970470108a
25 25
26 #if defined (HAVE_CONFIG_H) 26 #if defined (HAVE_CONFIG_H)
27 # include "config.h" 27 # include "config.h"
28 #endif 28 #endif
29 29
30 #include <atomic>
30 #include <csignal> 31 #include <csignal>
31 #include <cstdlib> 32 #include <cstdlib>
32 33
33 #include <iostream> 34 #include <iostream>
34 #include <new> 35 #include <new>
63 // Nonzero means we have already printed a message for this series of 64 // Nonzero means we have already printed a message for this series of
64 // SIGPIPES. We assume that the writer will eventually give up. 65 // SIGPIPES. We assume that the writer will eventually give up.
65 int pipe_handler_error_count = 0; 66 int pipe_handler_error_count = 0;
66 67
67 // TRUE means we can be interrupted. 68 // TRUE means we can be interrupted.
68 bool can_interrupt = false; 69 std::atomic<bool> can_interrupt{false};
69 70
70 // TRUE means we should try to enter the debugger on SIGINT. 71 // TRUE means we should try to enter the debugger on SIGINT.
71 bool Vdebug_on_interrupt = false; 72 bool Vdebug_on_interrupt = false;
72 73
73 // Allow users to avoid writing octave-workspace for SIGHUP (sent by 74 // Allow users to avoid writing octave-workspace for SIGHUP (sent by
80 81
81 // Similar to Vsighup_dumps_octave_core, but for SIGTERM signal. 82 // Similar to Vsighup_dumps_octave_core, but for SIGTERM signal.
82 static bool Vsigterm_dumps_octave_core = true; 83 static bool Vsigterm_dumps_octave_core = true;
83 84
84 // List of signals we have caught since last call to signal_handler. 85 // List of signals we have caught since last call to signal_handler.
85 static bool *signals_caught = nullptr; 86 static std::atomic<bool> *signals_caught = nullptr;
86 87
87 static void 88 static void
88 my_friendly_exit (int sig, bool save_vars = true) 89 my_friendly_exit (int sig, bool save_vars = true)
89 { 90 {
90 std::cerr << "fatal: caught signal " 91 std::cerr << "fatal: caught signal "
192 193
193 child_list& kids = __get_child_list__ (); 194 child_list& kids = __get_child_list__ ();
194 195
195 for (int sig = 0; sig < octave_num_signals (); sig++) 196 for (int sig = 0; sig < octave_num_signals (); sig++)
196 { 197 {
197 if (signals_caught[sig]) 198 bool expected = true;
199
200 if (signals_caught[sig].compare_exchange_strong (expected, false))
198 { 201 {
199 signals_caught[sig] = false;
200
201 if ((have_sigchld && sig == sigchld) 202 if ((have_sigchld && sig == sigchld)
202 || (have_sigcld && sig == sigcld)) 203 || (have_sigcld && sig == sigcld))
203 { 204 {
204 // FIXME: should we block or ignore? 205 // FIXME: should we block or ignore?
205 volatile interrupt_handler saved_interrupt_handler 206 volatile interrupt_handler saved_interrupt_handler
221 222
222 // Don't loop forever on account of this. 223 // Don't loop forever on account of this.
223 // FIXME: is this really needed? Does it do anything 224 // FIXME: is this really needed? Does it do anything
224 // useful now? 225 // useful now?
225 226
226 if (pipe_handler_error_count++ > 100 227 const int curr_pipe_handler_error_count = pipe_handler_error_count++;
227 && octave_interrupt_state >= 0) 228
228 octave_interrupt_state++; 229 if (curr_pipe_handler_error_count > 100)
230 {
231 sig_atomic_t curr_interrupt_state
232 = octave_interrupt_state.load();
233
234 sig_atomic_t new_interrupt_state;
235
236 do
237 new_interrupt_state = curr_interrupt_state + 1;
238 while (curr_interrupt_state >= 0 &&
239 ! octave_interrupt_state.compare_exchange_weak
240 (curr_interrupt_state, new_interrupt_state));
241 }
242
229 } 243 }
230 else if (have_sighup && sig == sighup) 244 else if (have_sighup && sig == sighup)
231 my_friendly_exit (sighup, Vsighup_dumps_octave_core); 245 my_friendly_exit (sighup, Vsighup_dumps_octave_core);
232 else if (have_sigquit && sig == sigquit) 246 else if (have_sigquit && sig == sigquit)
233 my_friendly_exit (sigquit, Vsigquit_dumps_octave_core); 247 my_friendly_exit (sigquit, Vsigquit_dumps_octave_core);
274 { 288 {
275 // FIXME: this function may execute in a separate signal handler or 289 // FIXME: this function may execute in a separate signal handler or
276 // signal watcher thread so it should probably be more careful about 290 // signal watcher thread so it should probably be more careful about
277 // how it accesses global objects. 291 // how it accesses global objects.
278 292
279 octave_signal_caught = 1; 293 octave_signal_caught = true;
280 294
281 signals_caught[sig] = true; 295 signals_caught[sig] = true;
282 296
283 static int sigint; 297 static int sigint;
284 static const bool have_sigint 298 static const bool have_sigint
294 if (! octave_initialized) 308 if (! octave_initialized)
295 exit (1); 309 exit (1);
296 310
297 if (can_interrupt) 311 if (can_interrupt)
298 { 312 {
299 octave_signal_caught = 1; 313 octave_signal_caught = true;
300 octave_interrupt_state++; 314 octave_interrupt_state++;
301 } 315 }
302 } 316 }
303 } 317 }
304 318
363 377
364 void 378 void
365 install_signal_handlers (void) 379 install_signal_handlers (void)
366 { 380 {
367 if (! signals_caught) 381 if (! signals_caught)
368 signals_caught = new bool [octave_num_signals ()]; 382 signals_caught = new std::atomic<bool> [octave_num_signals ()];
369 383
370 for (int i = 0; i < octave_num_signals (); i++) 384 for (int i = 0; i < octave_num_signals (); i++)
371 signals_caught[i] = false; 385 signals_caught[i] = false;
372 386
373 // Interrupt signals. 387 // Interrupt signals.