Mercurial > octave
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. |