Revert 1c82c481a5.

Problem with the test on macOS and FreeBSD + issue with downstream use
of signals_are_pending

Changes

@@ -189,9 +189,6 @@ Working version
 
 ### Other libraries:
 
-- GPR#2104, PR#4127, PR#7709: Fix Thread.sigmask.
-  (Jacques-Henri Jourdan, review by Jérémie Dimino)
-
 - GPR#1061: Add ?follow parameter to Unix.link. This allows hardlinking
   symlinks.
   (Christopher Zimmermann, review by Xavier Leroy, Damien Doligez, David

otherlibs/systhreads/st_posix.h

@@ -40,19 +40,10 @@ typedef int st_retcode;
 
 #define SIGPREEMPTION SIGVTALRM
 
-static void caml_get_sigmask_hook_pthread(int* mask) {
-  int i;
-  sigset_t set;
-  pthread_sigmask(/* dummy */ SIG_BLOCK, NULL, &set);
-  for (i = 0; i < NSIG; i++)
-    mask[i] = sigismember(&set, i);
-}
-
 /* OS-specific initialization */
 
 static int st_initialize(void)
 {
-  caml_get_sigmask_hook = caml_get_sigmask_hook_pthread;
   return 0;
 }
 

otherlibs/unix/signals.c

@@ -78,11 +78,7 @@ CAMLprim value unix_sigprocmask(value vaction, value vset)
 CAMLprim value unix_sigpending(value unit)
 {
   sigset_t pending;
-  int i;
   if (sigpending(&pending) == -1) uerror("sigpending", Nothing);
-  for (i = 1; i < NSIG; i++)
-    if(caml_pending_signals[i])
-      sigaddset(&pending, i);
   return encode_sigset(&pending);
 }
 

runtime/caml/signals.h

@@ -27,7 +27,7 @@ extern "C" {
 #endif
 
 #ifdef CAML_INTERNALS
-CAMLextern intnat volatile caml_signals_might_be_pending;
+CAMLextern intnat volatile caml_signals_are_pending;
 CAMLextern intnat volatile caml_pending_signals[];
 CAMLextern int volatile caml_something_to_do;
 extern int volatile caml_requested_major_slice;
@@ -39,7 +39,7 @@ CAMLextern int caml_convert_signal_number (int);
 CAMLextern int caml_rev_convert_signal_number (int);
 void caml_execute_signal(int signal_number, int in_signal_handler);
 void caml_record_signal(int signal_number);
-void caml_process_pending_signals();
+void caml_process_pending_signals(void);
 void caml_process_event(void);
 int caml_set_signal_action(int signo, int action);
 
@@ -47,7 +47,6 @@ CAMLextern void (*caml_enter_blocking_section_hook)(void);
 CAMLextern void (*caml_leave_blocking_section_hook)(void);
 CAMLextern int (*caml_try_leave_blocking_section_hook)(void);
 CAMLextern void (* volatile caml_async_action_hook)(void);
-CAMLextern void (*caml_get_sigmask_hook)(int*);
 #endif /* CAML_INTERNALS */
 
 CAMLextern void caml_enter_blocking_section (void);

runtime/signals.c

@@ -41,55 +41,24 @@
 
 /* The set of pending signals (received but not yet processed) */
 
-CAMLexport intnat volatile caml_signals_might_be_pending = 0;
+CAMLexport intnat volatile caml_signals_are_pending = 0;
 CAMLexport intnat volatile caml_pending_signals[NSIG];
 
-#ifdef POSIX_SIGNALS
-static void caml_get_sigmask_hook_default(int* mask) {
-  int i;
-  sigset_t set;
-  sigprocmask(/* dummy */ SIG_BLOCK, NULL, &set);
-  mask[0] = 0;
-  for (i = 1; i < NSIG; i++) mask[i] = sigismember(&set, i);
-}
-#else
-static void caml_get_sigmask_hook_default(int* mask) {
-  int i;
-  for (i = 0; i < NSIG; i++) mask[i] = 0;
-}
-#endif
-
-CAMLexport void (*caml_get_sigmask_hook)(int*) = caml_get_sigmask_hook_default;
-
 /* Execute all pending signals */
 
-void caml_process_pending_signals()
+void caml_process_pending_signals(void)
 {
   int i;
-  int blocked[NSIG];
-  int really_pending;
 
-  if(!caml_signals_might_be_pending)
-    return;
-  caml_signals_might_be_pending = 0;
-
-  /* Check that there is indeed a pending signal before issuing the
-     syscall in [caml_get_sigmask_hook]. */
-  really_pending = 0;
-  for (i = 0; i < NSIG; i++)
-    if (caml_pending_signals[i]) {
-      really_pending = 1;
-      break;
-    }
-  if(!really_pending)
-    return;
-
-  caml_get_sigmask_hook(blocked);
-  for (i = 0; i < NSIG; i++)
-    if (caml_pending_signals[i] && !blocked[i]) {
-      caml_pending_signals[i] = 0;
-      caml_execute_signal(i, 0);
+  if (caml_signals_are_pending) {
+    caml_signals_are_pending = 0;
+    for (i = 0; i < NSIG; i++) {
+      if (caml_pending_signals[i]) {
+        caml_pending_signals[i] = 0;
+        caml_execute_signal(i, 0);
+      }
     }
+  }
 }
 
 /* Record the delivery of a signal, and arrange for it to be processed
@@ -102,7 +71,7 @@ void caml_process_pending_signals()
 void caml_record_signal(int signal_number)
 {
   caml_pending_signals[signal_number] = 1;
-  caml_signals_might_be_pending = 1;
+  caml_signals_are_pending = 1;
 #ifndef NATIVE_CODE
   caml_something_to_do = 1;
 #else
@@ -148,7 +117,7 @@ CAMLexport void caml_enter_blocking_section(void)
     caml_enter_blocking_section_hook ();
     /* Check again for pending signals.
        If none, done; otherwise, try again */
-    if (! caml_signals_might_be_pending) break;
+    if (! caml_signals_are_pending) break;
     caml_leave_blocking_section_hook ();
   }
 }
@@ -159,22 +128,7 @@ CAMLexport void caml_leave_blocking_section(void)
   /* Save the value of errno (PR#5982). */
   saved_errno = errno;
   caml_leave_blocking_section_hook ();
-
-  /* Some other thread may have switched
-     [caml_signals_might_be_pending] to 0 even though there are still
-     pending signals (masked in the other thread). To handle this
-     case, we force re-examination of all signals by setting it back
-     to 1.
-
-     Another case where this is necessary (even in a single threaded
-     setting) is when the blocking section unmasks a pending signal:
-     If the signal is pending and masked but has already been
-     examinated by [caml_process_pending_signals], then
-     [caml_signals_might_be_pending] is 0 but the signal needs to be
-     handled at this point. */
-  caml_signals_might_be_pending = 1;
   caml_process_pending_signals();
-
   errno = saved_errno;
 }
 

testsuite/tests/lib-systhreads/ocamltests

@@ -1,3 +1,2 @@
 testfork.ml
 testpreempt.ml
-threadsigmask.ml

testsuite/tests/lib-systhreads/threadsigmask.ml

@@ -1,56 +0,0 @@
-(* TEST
-   (*
-     Thread.sigmask is not available on Windows
-   *)
-   include systhreads
-   * not-windows
-   ** bytecode
-   ** native
-*)
-
-exception Exc
-
-(* A computation that lasts at least 2 seconds after the initial time
-   given in time0 *)
-let rec long_computation time0 =
-  let rec generate_list n =
-    let rec aux acc = function
-      | 0 -> acc
-      | n -> aux (float n :: acc) (n-1)
-    in
-    aux [] n
-  in
-
-  let long_list = generate_list 100000 in
-  let res = List.length (List.rev_map sin long_list) in
-  if Sys.time () -. time0 > 2. then
-    Printf.printf "Long computation result: %d\n%!" res
-  else long_computation time0
-
-
-let thread n =
-  try long_computation (Sys.time ())
-  with Exc -> Printf.printf "Signal handled in thread %d\n%!" n
-
-(* The handler of the signal *)
-let interrupt signal =
-  raise Exc
-
-let _ =
-  (* Block the signal in all threads, except in the main thread. *)
-  ignore (Thread.sigmask Unix.SIG_BLOCK [Sys.sigalrm]);
-
-  (* Spawn the threads *)
-  ignore (Thread.create thread 1);
-  ignore (Thread.create thread 2);
-  ignore (Thread.create thread 3);
-
-  (* Unblock the signal in the main thread. *)
-  ignore (Thread.sigmask Unix.SIG_UNBLOCK [Sys.sigalrm]);
-
-  (* Setup the alarm *)
-  ignore (Unix.alarm 1);
-  Sys.set_signal Sys.sigalrm (Sys.Signal_handle interrupt);
-
-  (* Make sure the main thread does something *)
-  thread 0

testsuite/tests/lib-systhreads/threadsigmask.reference

@@ -1 +0,0 @@
-Signal handled in thread 0