[PATCH] Ignore pending_signals when checking for quits.

[PATCH] Ignore pending_signals when checking for quits.

[Philipp Stephani]

pending_signals is often set if no quit is pending.  This results in
bugs in module code if the module returns but no quit is actually
pending.

* src/emacs-module.c (module_should_quit): Use QUITP macro to check
whether the caller should quit.

* src/eval.c: Remove obsolete comment.

* test/data/emacs-module/mod-test.c (signal_wrong_type_argument)
(signal_errno): New helper functions.
(Fmod_test_sleep_until): New test module function.

* test/src/emacs-module-tests.el (mod-test-sleep-until): New unit
test.
---
 src/emacs-module.c                |  6 +--
 src/eval.c                        |  5 +--
 test/data/emacs-module/mod-test.c | 65 ++++++++++++++++++++++++++++++-
 test/src/emacs-module-tests.el    | 17 ++++++++
 4 files changed, 85 insertions(+), 8 deletions(-)

diff --git a/src/emacs-module.c b/src/emacs-module.c
index e695a3d2e6..a00487137c 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -671,13 +671,13 @@ module_vec_size (emacs_env *env, emacs_value vec)
   return ASIZE (lvec);
 }
 
-/* This function should return true if and only if maybe_quit would do
-   anything.  */
+/* This function should return true if and only if maybe_quit would
+   quit.  */
 static bool
 module_should_quit (emacs_env *env)
 {
   MODULE_FUNCTION_BEGIN_NO_CATCH (false);
-  return (! NILP (Vquit_flag) && NILP (Vinhibit_quit)) || pending_signals;
+  return QUITP;
 }
 
 \f
diff --git a/src/eval.c b/src/eval.c
index c64a40b955..a73477a581 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1572,10 +1572,7 @@ process_quit_flag (void)
    If quit-flag is set to `kill-emacs' the SIGINT handler has received
    a request to exit Emacs when it is safe to do.
 
-   When not quitting, process any pending signals.
-
-   If you change this function, also adapt module_should_quit in
-   emacs-module.c.  */
+   When not quitting, process any pending signals.  */
 
 void
 maybe_quit (void)
diff --git a/test/data/emacs-module/mod-test.c b/test/data/emacs-module/mod-test.c
index 98242e85ba..dddd0cddbc 100644
--- a/test/data/emacs-module/mod-test.c
+++ b/test/data/emacs-module/mod-test.c
@@ -18,9 +18,13 @@ You should have received a copy of the GNU General Public License
 along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 
 #include <assert.h>
+#include <errno.h>
+#include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <limits.h>
+#include <string.h>
+#include <time.h>
+
 #include <emacs-module.h>
 
 int plugin_is_GPL_compatible;
@@ -299,6 +303,64 @@ Fmod_test_invalid_finalizer (emacs_env *env, ptrdiff_t nargs, emacs_value *args,
   return env->funcall (env, env->intern (env, "garbage-collect"), 0, NULL);
 }
 
+static void
+signal_wrong_type_argument (emacs_env *env, const char *predicate,
+                            emacs_value arg)
+{
+  emacs_value symbol = env->intern (env, "wrong-type-argument");
+  emacs_value elements[2] = {env->intern (env, predicate), arg};
+  emacs_value data = env->funcall (env, env->intern (env, "list"), 2, elements);
+  env->non_local_exit_signal (env, symbol, data);
+}
+
+static void
+signal_errno (emacs_env *env, const char *function)
+{
+  const char *message = strerror (errno);
+  emacs_value message_value = env->make_string (env, message, strlen (message));
+  emacs_value symbol = env->intern (env, "file-error");
+  emacs_value elements[2]
+    = {env->make_string (env, function, strlen (function)), message_value};
+  emacs_value data = env->funcall (env, env->intern (env, "list"), 2, elements);
+  env->non_local_exit_signal (env, symbol, data);
+}
+
+/* A long-running operation that occasionally calls `should_quit'.  */
+
+static emacs_value
+Fmod_test_sleep_until (emacs_env *env, ptrdiff_t nargs, emacs_value *args,
+                       void *data)
+{
+  assert (nargs == 1);
+  double until = env->extract_float (env, args[0]);
+  if (env->non_local_exit_check (env))
+    return NULL;
+  if (until <= 0)
+    {
+      signal_wrong_type_argument (env, "cl-plusp", args[0]);
+      return NULL;
+    }
+  while (true)
+    {
+      struct timespec now;
+      if (clock_gettime (CLOCK_REALTIME, &now))
+        {
+          signal_errno (env, "clock_gettime");
+          return NULL;
+        }
+      if (now.tv_sec + (double) now.tv_nsec / 1e9 >= until)
+        break;
+      struct timespec amount = {.tv_sec = 0, .tv_nsec = 10000000};
+      if (nanosleep (&amount, NULL) && errno != EINTR)
+        {
+          signal_errno (env, "nanosleep");
+          return NULL;
+        }
+      if (env->should_quit (env))
+        return NULL;
+    }
+  return env->intern (env, "finished");
+}
 
 /* Lisp utilities for easier readability (simple wrappers).  */
 
@@ -367,6 +429,7 @@ emacs_module_init (struct emacs_runtime *ert)
   DEFUN ("mod-test-invalid-load", Fmod_test_invalid_load, 0, 0, NULL, NULL);
   DEFUN ("mod-test-invalid-finalizer", Fmod_test_invalid_finalizer, 0, 0,
          NULL, NULL);
+  DEFUN ("mod-test-sleep-until", Fmod_test_sleep_until, 1, 1, NULL, NULL);
 
 #undef DEFUN
 
diff --git a/test/src/emacs-module-tests.el b/test/src/emacs-module-tests.el
index e4593044ec..9334f951fe 100644
--- a/test/src/emacs-module-tests.el
+++ b/test/src/emacs-module-tests.el
@@ -289,4 +289,21 @@ module--test-assertion
     (should (member '(provide . mod-test) entries))
     (should (member '(defun . mod-test-sum) entries))))
 
+(ert-deftest mod-test-sleep-until ()
+  "Check that `mod-test-sleep-until' either returns normally or quits.
+Interactively, you can try hitting \\[keyboard-quit] to quit."
+  ;; Guard against some caller setting `inhibit-quit'.
+  (with-local-quit
+    (condition-case nil
+        (should (eq (with-local-quit
+                      ;; Because `inhibit-quit' is nil here, the next
+                      ;; form either quits or returns `finished'.
+                      (mod-test-sleep-until
+                       ;; Interactively, run for 5 seconds to give the
+                       ;; user time to quit.  In batch mode, run only
+                       ;; briefly since the user can't quit.
+                       (float-time (time-add nil (if noninteractive 0.1 5)))))
+                    'finished))
+      (quit))))
+
 ;;; emacs-module-tests.el ends here
-- 
2.17.2 (Apple Git-113)

Re: [PATCH] Ignore pending_signals when checking for quits.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Wed,  2 Jan 2019 22:22:18 +0100
> Cc: Philipp Stephani <phst@google.com>
> 
>  static bool
>  module_should_quit (emacs_env *env)
>  {
>    MODULE_FUNCTION_BEGIN_NO_CATCH (false);
> -  return (! NILP (Vquit_flag) && NILP (Vinhibit_quit)) || pending_signals;
> +  return QUITP;
>  }

Bother.  I see your point regarding the return value when just
pending_signals is set, but disregarding pending_signals doesn't sound
TRT to me, either.  It means various Emacs features based on input
detection won't work while the module code runs, even if the module
tries to be nice and does call module_should_quit.  For example,
while-no-input and atimers won't work, and Emacs will generally be
much less responsive to user input.

So maybe we should indeed return true only if QUITP says so, but we
should also call process_pending_signals from module_should_quit, when
pending_signals is non-zero?

Re: [PATCH] Ignore pending_signals when checking for quits.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Wed,  2 Jan 2019 22:22:18 +0100
> Cc: Philipp Stephani <phst@google.com>

One comment about the new module test:

> +      struct timespec now;
> +      if (clock_gettime (CLOCK_REALTIME, &now))

clock_gettime is not portable enough; please use gettime (from Gnulib;
see lib/gettime.c), it will call clock_gettime where it's available.

Thanks.

Re: [PATCH] Ignore pending_signals when checking for quits.

[Philipp Stephani]

Am Do., 3. Jan. 2019 um 15:03 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Wed,  2 Jan 2019 22:22:18 +0100
> > Cc: Philipp Stephani <phst@google.com>
> >
> >  static bool
> >  module_should_quit (emacs_env *env)
> >  {
> >    MODULE_FUNCTION_BEGIN_NO_CATCH (false);
> > -  return (! NILP (Vquit_flag) && NILP (Vinhibit_quit)) || pending_signals;
> > +  return QUITP;
> >  }
>
> Bother.  I see your point regarding the return value when just
> pending_signals is set, but disregarding pending_signals doesn't sound
> TRT to me, either.  It means various Emacs features based on input
> detection won't work while the module code runs, even if the module
> tries to be nice and does call module_should_quit.  For example,
> while-no-input and atimers won't work, and Emacs will generally be
> much less responsive to user input.
>
> So maybe we should indeed return true only if QUITP says so, but we
> should also call process_pending_signals from module_should_quit, when
> pending_signals is non-zero?

Wouldn't that mean that Emacs could do something (e.g. process
events)? That wouldn't match the naming and intention of should_quit:
By its name, it should only query information and not change any
state.

Re: [PATCH] Ignore pending_signals when checking for quits.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sun, 10 Feb 2019 19:49:39 +0100
> Cc: Emacs developers <emacs-devel@gnu.org>, Philipp Stephani <phst@google.com>
> 
> > Bother.  I see your point regarding the return value when just
> > pending_signals is set, but disregarding pending_signals doesn't sound
> > TRT to me, either.  It means various Emacs features based on input
> > detection won't work while the module code runs, even if the module
> > tries to be nice and does call module_should_quit.  For example,
> > while-no-input and atimers won't work, and Emacs will generally be
> > much less responsive to user input.
> >
> > So maybe we should indeed return true only if QUITP says so, but we
> > should also call process_pending_signals from module_should_quit, when
> > pending_signals is non-zero?
> 
> Wouldn't that mean that Emacs could do something (e.g. process
> events)? That wouldn't match the naming and intention of should_quit:
> By its name, it should only query information and not change any
> state.

If the only problem is the name, we could change the name.  Or we
could introduce a new function.  But let's first agree about the
substance: a well-behaving module should from time to time call
process_pending_signals.  Agreed?

Re: [PATCH] Ignore pending_signals when checking for quits.

[Philipp Stephani]

Am So., 10. Feb. 2019 um 20:41 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sun, 10 Feb 2019 19:49:39 +0100
> > Cc: Emacs developers <emacs-devel@gnu.org>, Philipp Stephani <phst@google.com>
> >
> > > Bother.  I see your point regarding the return value when just
> > > pending_signals is set, but disregarding pending_signals doesn't sound
> > > TRT to me, either.  It means various Emacs features based on input
> > > detection won't work while the module code runs, even if the module
> > > tries to be nice and does call module_should_quit.  For example,
> > > while-no-input and atimers won't work, and Emacs will generally be
> > > much less responsive to user input.
> > >
> > > So maybe we should indeed return true only if QUITP says so, but we
> > > should also call process_pending_signals from module_should_quit, when
> > > pending_signals is non-zero?
> >
> > Wouldn't that mean that Emacs could do something (e.g. process
> > events)? That wouldn't match the naming and intention of should_quit:
> > By its name, it should only query information and not change any
> > state.
>
> If the only problem is the name, we could change the name.  Or we
> could introduce a new function.  But let's first agree about the
> substance: a well-behaving module should from time to time call
> process_pending_signals.  Agreed?

Yes, absolutely. We can't change the name (this would break backwards
compatibility), but introducing a new function would be fine.
For should_quit itself, I still think we should make the change in
this patch. The current code is clearly buggy: it promises to quit but
doesn't in most cases.

Re: [PATCH] Ignore pending_signals when checking for quits.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sun, 10 Feb 2019 20:46:22 +0100
> Cc: Emacs developers <emacs-devel@gnu.org>, Philipp Stephani <phst@google.com>
> 
> > If the only problem is the name, we could change the name.  Or we
> > could introduce a new function.  But let's first agree about the
> > substance: a well-behaving module should from time to time call
> > process_pending_signals.  Agreed?
> 
> Yes, absolutely. We can't change the name (this would break backwards
> compatibility), but introducing a new function would be fine.
> For should_quit itself, I still think we should make the change in
> this patch. The current code is clearly buggy: it promises to quit but
> doesn't in most cases.

I'm okay with such a change if it will be accompanied by introduction
of that new function and suitable changes to the manual which explains
why they are needed.

[PATCH] Ignore pending_signals when checking for quits.

[Philipp Stephani]

pending_signals is often set if no quit is pending.  This results in
bugs in module code if the module returns but no quit is actually
pending.

As a better alternative, add a new process_input environment function
for Emacs 27.  That function processes signals (like maybe_quit).

* configure.ac: Add module snippet for Emacs 27.

* src/module-env-27.h: New file.

* src/emacs-module.h.in: Add process_input function to environment
interface.

* src/emacs-module.c (module_should_quit): Use QUITP macro to check
whether the caller should quit.
(module_process_input): New function.
(initialize_environment): Use it.

* src/eval.c: Remove obsolete comment.

* test/data/emacs-module/mod-test.c (signal_wrong_type_argument)
(signal_errno): New helper functions.
(Fmod_test_sleep_until): New test module function.

* test/src/emacs-module-tests.el (mod-test-sleep-until): New unit
test.

* doc/lispref/internals.texi (Module Misc): Document process_input.
---
 configure.ac                      |  2 +
 doc/lispref/internals.texi        | 22 +++++++++-
 etc/NEWS                          |  3 ++
 src/emacs-module.c                | 15 +++++--
 src/emacs-module.h.in             | 21 +++++++++-
 src/eval.c                        |  5 +--
 src/module-env-27.h               |  4 ++
 test/data/emacs-module/mod-test.c | 69 ++++++++++++++++++++++++++++++-
 test/src/emacs-module-tests.el    | 20 +++++++++
 9 files changed, 151 insertions(+), 10 deletions(-)
 create mode 100644 src/module-env-27.h

diff --git a/configure.ac b/configure.ac
index 58579008f3..3318f04a5a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3689,8 +3689,10 @@ AC_DEFUN
 AC_CONFIG_FILES([src/emacs-module.h])
 AC_SUBST_FILE([module_env_snippet_25])
 AC_SUBST_FILE([module_env_snippet_26])
+AC_SUBST_FILE([module_env_snippet_27])
 module_env_snippet_25="$srcdir/src/module-env-25.h"
 module_env_snippet_26="$srcdir/src/module-env-26.h"
+module_env_snippet_27="$srcdir/src/module-env-27.h"
 
 ### Use -lpng if available, unless '--with-png=no'.
 HAVE_PNG=no
diff --git a/doc/lispref/internals.texi b/doc/lispref/internals.texi
index 3fbff266ad..56465126f4 100644
--- a/doc/lispref/internals.texi
+++ b/doc/lispref/internals.texi
@@ -1623,7 +1623,27 @@ Module Misc
 @deftypefn Function bool should_quit (emacs_env *@var{env})
 This function returns @code{true} if the user wants to quit.  In that
 case, we recommend that your module function aborts any on-going
-processing and returns as soon as possible.
+processing and returns as soon as possible.  In most cases, use
+@code{process_input} instead.
+@end deftypefn
+
+To process input events in addition to checking whether the user wants
+to quit, use the following function, which is available since Emacs
+27.1.
+
+@anchor{process_input}
+@deftypefn Function enum emacs_process_input_result process_input (emacs_env *@var{env})
+This function processes pending input events.  It returns
+@code{emacs_process_input_quit} if the user wants to quit or an error
+occurred while processing signals.  In that case, we recommend that
+your module function aborts any on-going processing and returns as
+soon as possible.  If the module code may continue running,
+@code{process_input} returns @code{emacs_process_input_continue}.  The
+return value is @code{emacs_process_input_continue} if and only if
+there is no pending nonlocal exit in @code{env}.  If the module
+continues after calling @code{process_input}, global state such as
+variable values and buffer content may have been modified in arbitrary
+ways.
 @end deftypefn
 
 @node Module Nonlocal
diff --git a/etc/NEWS b/etc/NEWS
index 75c8dc0b8e..08e2e0c728 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1564,6 +1564,9 @@ given frame supports resizing.
 This is currently supported on GNUish hosts and on modern versions of
 MS-Windows.
 
+** New module environment function 'process_input' to process user
+input while module code is running.
+
 \f
 * Changes in Emacs 27.1 on Non-Free Operating Systems
 
diff --git a/src/emacs-module.c b/src/emacs-module.c
index cbab023420..b70d6cea81 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -671,13 +671,21 @@ module_vec_size (emacs_env *env, emacs_value vec)
   return ASIZE (lvec);
 }
 
-/* This function should return true if and only if maybe_quit would do
-   anything.  */
+/* This function should return true if and only if maybe_quit would
+   quit.  */
 static bool
 module_should_quit (emacs_env *env)
 {
   MODULE_FUNCTION_BEGIN_NO_CATCH (false);
-  return (! NILP (Vquit_flag) && NILP (Vinhibit_quit)) || pending_signals;
+  return QUITP;
+}
+
+static enum emacs_process_input_result
+module_process_input (emacs_env *env)
+{
+  MODULE_FUNCTION_BEGIN (emacs_process_input_quit);
+  maybe_quit ();
+  return emacs_process_input_continue;
 }
 
 \f
@@ -1082,6 +1090,7 @@ initialize_environment (emacs_env *env, struct emacs_env_private *priv)
   env->vec_get = module_vec_get;
   env->vec_size = module_vec_size;
   env->should_quit = module_should_quit;
+  env->process_input = module_process_input;
   Vmodule_environments = Fcons (make_mint_ptr (env), Vmodule_environments);
   return env;
 }
diff --git a/src/emacs-module.h.in b/src/emacs-module.h.in
index 4c5286f625..009d1583fe 100644
--- a/src/emacs-module.h.in
+++ b/src/emacs-module.h.in
@@ -47,7 +47,7 @@ extern "C" {
 #endif
 
 /* Current environment.  */
-typedef struct emacs_env_26 emacs_env;
+typedef struct emacs_env_27 emacs_env;
 
 /* Opaque pointer representing an Emacs Lisp value.
    BEWARE: Do not assume NULL is a valid value!  */
@@ -83,6 +83,16 @@ enum emacs_funcall_exit
   emacs_funcall_exit_throw = 2
 };
 
+/* Possible return values for emacs_env.process_input.  */
+enum emacs_process_input_result
+{
+  /* Module code may continue  */
+  emacs_process_input_continue = 0,
+
+  /* Module code should return control to Emacs as soon as possible.  */
+  emacs_process_input_quit = 1
+};
+
 struct emacs_env_25
 {
 @module_env_snippet_25@
@@ -95,6 +105,15 @@ struct emacs_env_26
 @module_env_snippet_26@
 };
 
+struct emacs_env_27
+{
+@module_env_snippet_25@
+
+@module_env_snippet_26@
+
+@module_env_snippet_27@
+};
+
 /* Every module should define a function as follows.  */
 extern int emacs_module_init (struct emacs_runtime *ert)
   EMACS_NOEXCEPT
diff --git a/src/eval.c b/src/eval.c
index b094fc2e66..b6cdfc911d 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1575,10 +1575,7 @@ process_quit_flag (void)
    If quit-flag is set to `kill-emacs' the SIGINT handler has received
    a request to exit Emacs when it is safe to do.
 
-   When not quitting, process any pending signals.
-
-   If you change this function, also adapt module_should_quit in
-   emacs-module.c.  */
+   When not quitting, process any pending signals.  */
 
 void
 maybe_quit (void)
diff --git a/src/module-env-27.h b/src/module-env-27.h
new file mode 100644
index 0000000000..b491b60fbb
--- /dev/null
+++ b/src/module-env-27.h
@@ -0,0 +1,4 @@
+  /* Processes pending input events and returns whether the module
+     function should quit.  */
+  enum emacs_process_input_result (*process_input) (emacs_env *env)
+    EMACS_ATTRIBUTE_NONNULL (1);
diff --git a/test/data/emacs-module/mod-test.c b/test/data/emacs-module/mod-test.c
index 98242e85ba..47ea159d0e 100644
--- a/test/data/emacs-module/mod-test.c
+++ b/test/data/emacs-module/mod-test.c
@@ -17,12 +17,20 @@ GNU General Public License for more details.
 You should have received a copy of the GNU General Public License
 along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 
+#include "config.h"
+
 #include <assert.h>
+#include <errno.h>
+#include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <limits.h>
+#include <string.h>
+#include <time.h>
+
 #include <emacs-module.h>
 
+#include "timespec.h"
+
 int plugin_is_GPL_compatible;
 
 #if INTPTR_MAX <= 0
@@ -299,6 +307,64 @@ Fmod_test_invalid_finalizer (emacs_env *env, ptrdiff_t nargs, emacs_value *args,
   return env->funcall (env, env->intern (env, "garbage-collect"), 0, NULL);
 }
 
+static void
+signal_wrong_type_argument (emacs_env *env, const char *predicate,
+                            emacs_value arg)
+{
+  emacs_value symbol = env->intern (env, "wrong-type-argument");
+  emacs_value elements[2] = {env->intern (env, predicate), arg};
+  emacs_value data = env->funcall (env, env->intern (env, "list"), 2, elements);
+  env->non_local_exit_signal (env, symbol, data);
+}
+
+static void
+signal_errno (emacs_env *env, const char *function)
+{
+  const char *message = strerror (errno);
+  emacs_value message_value = env->make_string (env, message, strlen (message));
+  emacs_value symbol = env->intern (env, "file-error");
+  emacs_value elements[2]
+    = {env->make_string (env, function, strlen (function)), message_value};
+  emacs_value data = env->funcall (env, env->intern (env, "list"), 2, elements);
+  env->non_local_exit_signal (env, symbol, data);
+}
+
+/* A long-running operation that occasionally calls `should_quit' or
+   `process_input'.  */
+
+static emacs_value
+Fmod_test_sleep_until (emacs_env *env, ptrdiff_t nargs, emacs_value *args,
+                       void *data)
+{
+  assert (nargs == 2);
+  const double until_seconds = env->extract_float (env, args[0]);
+  if (env->non_local_exit_check (env))
+    return NULL;
+  if (until_seconds <= 0)
+    {
+      signal_wrong_type_argument (env, "cl-plusp", args[0]);
+      return NULL;
+    }
+  const bool process_input = env->is_not_nil (env, args[1]);
+  const struct timespec until = dtotimespec (until_seconds);
+  const struct timespec amount = make_timespec(0,  10000000);
+  while (true)
+    {
+      const struct timespec now = current_timespec ();
+      if (timespec_cmp (now, until) >= 0)
+        break;
+      if (nanosleep (&amount, NULL) && errno != EINTR)
+        {
+          signal_errno (env, "nanosleep");
+          return NULL;
+        }
+      if ((process_input
+           && env->process_input (env) == emacs_process_input_quit)
+          || env->should_quit (env))
+        return NULL;
+    }
+  return env->intern (env, "finished");
+}
 
 /* Lisp utilities for easier readability (simple wrappers).  */
 
@@ -367,6 +433,7 @@ emacs_module_init (struct emacs_runtime *ert)
   DEFUN ("mod-test-invalid-load", Fmod_test_invalid_load, 0, 0, NULL, NULL);
   DEFUN ("mod-test-invalid-finalizer", Fmod_test_invalid_finalizer, 0, 0,
          NULL, NULL);
+  DEFUN ("mod-test-sleep-until", Fmod_test_sleep_until, 2, 2, NULL, NULL);
 
 #undef DEFUN
 
diff --git a/test/src/emacs-module-tests.el b/test/src/emacs-module-tests.el
index e4593044ec..e30980b599 100644
--- a/test/src/emacs-module-tests.el
+++ b/test/src/emacs-module-tests.el
@@ -289,4 +289,24 @@ module--test-assertion
     (should (member '(provide . mod-test) entries))
     (should (member '(defun . mod-test-sum) entries))))
 
+(ert-deftest mod-test-sleep-until ()
+  "Check that `mod-test-sleep-until' either returns normally or quits.
+Interactively, you can try hitting \\[keyboard-quit] to quit."
+  (dolist (arg '(nil t))
+    ;; Guard against some caller setting `inhibit-quit'.
+    (with-local-quit
+      (condition-case nil
+          (should (eq (with-local-quit
+                        ;; Because `inhibit-quit' is nil here, the next
+                        ;; form either quits or returns `finished'.
+                        (mod-test-sleep-until
+                         ;; Interactively, run for 5 seconds to give the
+                         ;; user time to quit.  In batch mode, run only
+                         ;; briefly since the user can't quit.
+                         (float-time (time-add nil (if noninteractive 0.1 5)))
+                         ;; should_quit or process_input
+                         arg))
+                      'finished))
+        (quit)))))
+
 ;;; emacs-module-tests.el ends here
-- 
2.17.2 (Apple Git-113)

Re: [PATCH] Ignore pending_signals when checking for quits.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Cc: Philipp Stephani <phst@google.com>
> Date: Mon, 11 Feb 2019 21:17:39 +0100
> 
> pending_signals is often set if no quit is pending.  This results in
> bugs in module code if the module returns but no quit is actually
> pending.
> 
> As a better alternative, add a new process_input environment function
> for Emacs 27.  That function processes signals (like maybe_quit).

Thanks, this LGTM.

Re: [PATCH] Ignore pending_signals when checking for quits.

[Philipp Stephani]

Am Mi., 13. Feb. 2019 um 16:29 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Cc: Philipp Stephani <phst@google.com>
> > Date: Mon, 11 Feb 2019 21:17:39 +0100
> >
> > pending_signals is often set if no quit is pending.  This results in
> > bugs in module code if the module returns but no quit is actually
> > pending.
> >
> > As a better alternative, add a new process_input environment function
> > for Emacs 27.  That function processes signals (like maybe_quit).
>
> Thanks, this LGTM.

Thanks, pushed as 72ec233f2a.

Re: [PATCH] Ignore pending_signals when checking for quits.

[Philipp Stephani]

Am So., 24. Feb. 2019 um 22:51 Uhr schrieb Philipp Stephani
<p.stephani2@gmail.com>:
>
> Am Mi., 13. Feb. 2019 um 16:29 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
> >
> > > From: Philipp Stephani <p.stephani2@gmail.com>
> > > Cc: Philipp Stephani <phst@google.com>
> > > Date: Mon, 11 Feb 2019 21:17:39 +0100
> > >
> > > pending_signals is often set if no quit is pending.  This results in
> > > bugs in module code if the module returns but no quit is actually
> > > pending.
> > >
> > > As a better alternative, add a new process_input environment function
> > > for Emacs 27.  That function processes signals (like maybe_quit).
> >
> > Thanks, this LGTM.
>
> Thanks, pushed as 72ec233f2a.

Question: Would it be OK to backport the first part (using QUITP in
module_should_quit) to Emacs 26.3? Right now should_quit behaves
erratically: it occasionally quits, but in other instances doesn't
because pending_signals has been reset in the meantime.

Re: [PATCH] Ignore pending_signals when checking for quits.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Fri, 19 Apr 2019 21:06:48 +0200
> Cc: Emacs developers <emacs-devel@gnu.org>, Philipp Stephani <phst@google.com>
> 
> Question: Would it be OK to backport the first part (using QUITP in
> module_should_quit) to Emacs 26.3?

Please show the part that you suggest to backport, I don't think I
understand what that is.

Re: [PATCH] Ignore pending_signals when checking for quits.

[Philipp Stephani]

Am Fr., 19. Apr. 2019 um 22:16 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Fri, 19 Apr 2019 21:06:48 +0200
> > Cc: Emacs developers <emacs-devel@gnu.org>, Philipp Stephani <phst@google.com>
> >
> > Question: Would it be OK to backport the first part (using QUITP in
> > module_should_quit) to Emacs 26.3?
>
> Please show the part that you suggest to backport, I don't think I
> understand what that is.

The initial patch, i.e.

--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -671,13 +671,13 @@ module_vec_size (emacs_env *env, emacs_value vec)
   return ASIZE (lvec);
 }

-/* This function should return true if and only if maybe_quit would do
-   anything.  */
+/* This function should return true if and only if maybe_quit would
+   quit.  */
 static bool
 module_should_quit (emacs_env *env)
 {
   MODULE_FUNCTION_BEGIN_NO_CATCH (false);
-  return (! NILP (Vquit_flag) && NILP (Vinhibit_quit)) || pending_signals;
+  return QUITP;
 }

Re: [PATCH] Ignore pending_signals when checking for quits.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sun, 28 Apr 2019 20:19:42 +0200
> Cc: Emacs developers <emacs-devel@gnu.org>, Philipp Stephani <phst@google.com>
> 
> > > Question: Would it be OK to backport the first part (using QUITP in
> > > module_should_quit) to Emacs 26.3?
> >
> > Please show the part that you suggest to backport, I don't think I
> > understand what that is.
> 
> The initial patch, i.e.
> 
> --- a/src/emacs-module.c
> +++ b/src/emacs-module.c
> @@ -671,13 +671,13 @@ module_vec_size (emacs_env *env, emacs_value vec)
>    return ASIZE (lvec);
>  }
> 
> -/* This function should return true if and only if maybe_quit would do
> -   anything.  */
> +/* This function should return true if and only if maybe_quit would
> +   quit.  */
>  static bool
>  module_should_quit (emacs_env *env)
>  {
>    MODULE_FUNCTION_BEGIN_NO_CATCH (false);
> -  return (! NILP (Vquit_flag) && NILP (Vinhibit_quit)) || pending_signals;
> +  return QUITP;
>  }

If you don't mind breaking compatibility in a minor release, I'm okay
with backporting this.

Re: [PATCH] Ignore pending_signals when checking for quits.

[Philipp Stephani]

Am So., 28. Apr. 2019 um 20:42 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sun, 28 Apr 2019 20:19:42 +0200
> > Cc: Emacs developers <emacs-devel@gnu.org>, Philipp Stephani <phst@google.com>
> >
> > > > Question: Would it be OK to backport the first part (using QUITP in
> > > > module_should_quit) to Emacs 26.3?
> > >
> > > Please show the part that you suggest to backport, I don't think I
> > > understand what that is.
> >
> > The initial patch, i.e.
> >
> > --- a/src/emacs-module.c
> > +++ b/src/emacs-module.c
> > @@ -671,13 +671,13 @@ module_vec_size (emacs_env *env, emacs_value vec)
> >    return ASIZE (lvec);
> >  }
> >
> > -/* This function should return true if and only if maybe_quit would do
> > -   anything.  */
> > +/* This function should return true if and only if maybe_quit would
> > +   quit.  */
> >  static bool
> >  module_should_quit (emacs_env *env)
> >  {
> >    MODULE_FUNCTION_BEGIN_NO_CATCH (false);
> > -  return (! NILP (Vquit_flag) && NILP (Vinhibit_quit)) || pending_signals;
> > +  return QUITP;
> >  }
>
> If you don't mind breaking compatibility in a minor release, I'm okay
> with backporting this.

Done (commit b83f83ccd4).