* [PATCH] Ignore pending_signals when checking for quits. @ 2019-01-02 21:22 Philipp Stephani 2019-01-03 14:03 ` Eli Zaretskii 2019-01-03 14:09 ` Eli Zaretskii 0 siblings, 2 replies; 15+ messages in thread From: Philipp Stephani @ 2019-01-02 21:22 UTC (permalink / raw) To: emacs-devel; +Cc: 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) ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] Ignore pending_signals when checking for quits. 2019-01-02 21:22 [PATCH] Ignore pending_signals when checking for quits Philipp Stephani @ 2019-01-03 14:03 ` Eli Zaretskii 2019-02-10 18:49 ` Philipp Stephani 2019-01-03 14:09 ` Eli Zaretskii 1 sibling, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2019-01-03 14:03 UTC (permalink / raw) To: Philipp Stephani; +Cc: phst, emacs-devel > 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? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Ignore pending_signals when checking for quits. 2019-01-03 14:03 ` Eli Zaretskii @ 2019-02-10 18:49 ` Philipp Stephani 2019-02-10 19:40 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Philipp Stephani @ 2019-02-10 18:49 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Philipp Stephani, Emacs developers 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Ignore pending_signals when checking for quits. 2019-02-10 18:49 ` Philipp Stephani @ 2019-02-10 19:40 ` Eli Zaretskii 2019-02-10 19:46 ` Philipp Stephani 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2019-02-10 19:40 UTC (permalink / raw) To: Philipp Stephani; +Cc: phst, emacs-devel > 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? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Ignore pending_signals when checking for quits. 2019-02-10 19:40 ` Eli Zaretskii @ 2019-02-10 19:46 ` Philipp Stephani 2019-02-10 20:15 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Philipp Stephani @ 2019-02-10 19:46 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Philipp Stephani, Emacs developers 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Ignore pending_signals when checking for quits. 2019-02-10 19:46 ` Philipp Stephani @ 2019-02-10 20:15 ` Eli Zaretskii 2019-02-11 20:17 ` Philipp Stephani 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2019-02-10 20:15 UTC (permalink / raw) To: Philipp Stephani; +Cc: phst, emacs-devel > 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] Ignore pending_signals when checking for quits. 2019-02-10 20:15 ` Eli Zaretskii @ 2019-02-11 20:17 ` Philipp Stephani 2019-02-13 15:28 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Philipp Stephani @ 2019-02-11 20:17 UTC (permalink / raw) To: emacs-devel, eliz; +Cc: 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) ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] Ignore pending_signals when checking for quits. 2019-02-11 20:17 ` Philipp Stephani @ 2019-02-13 15:28 ` Eli Zaretskii 2019-02-24 21:51 ` Philipp Stephani 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2019-02-13 15:28 UTC (permalink / raw) To: Philipp Stephani; +Cc: phst, emacs-devel > 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Ignore pending_signals when checking for quits. 2019-02-13 15:28 ` Eli Zaretskii @ 2019-02-24 21:51 ` Philipp Stephani 2019-04-19 19:06 ` Philipp Stephani 0 siblings, 1 reply; 15+ messages in thread From: Philipp Stephani @ 2019-02-24 21:51 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Philipp Stephani, Emacs developers 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Ignore pending_signals when checking for quits. 2019-02-24 21:51 ` Philipp Stephani @ 2019-04-19 19:06 ` Philipp Stephani 2019-04-19 20:15 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Philipp Stephani @ 2019-04-19 19:06 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Philipp Stephani, Emacs developers 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Ignore pending_signals when checking for quits. 2019-04-19 19:06 ` Philipp Stephani @ 2019-04-19 20:15 ` Eli Zaretskii 2019-04-28 18:19 ` Philipp Stephani 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2019-04-19 20:15 UTC (permalink / raw) To: Philipp Stephani; +Cc: phst, emacs-devel > 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Ignore pending_signals when checking for quits. 2019-04-19 20:15 ` Eli Zaretskii @ 2019-04-28 18:19 ` Philipp Stephani 2019-04-28 18:42 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Philipp Stephani @ 2019-04-28 18:19 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Philipp Stephani, Emacs developers 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; } ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Ignore pending_signals when checking for quits. 2019-04-28 18:19 ` Philipp Stephani @ 2019-04-28 18:42 ` Eli Zaretskii 2019-08-07 16:22 ` Philipp Stephani 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2019-04-28 18:42 UTC (permalink / raw) To: Philipp Stephani; +Cc: phst, emacs-devel > 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Ignore pending_signals when checking for quits. 2019-04-28 18:42 ` Eli Zaretskii @ 2019-08-07 16:22 ` Philipp Stephani 0 siblings, 0 replies; 15+ messages in thread From: Philipp Stephani @ 2019-08-07 16:22 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Philipp Stephani, Emacs developers 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). ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Ignore pending_signals when checking for quits. 2019-01-02 21:22 [PATCH] Ignore pending_signals when checking for quits Philipp Stephani 2019-01-03 14:03 ` Eli Zaretskii @ 2019-01-03 14:09 ` Eli Zaretskii 1 sibling, 0 replies; 15+ messages in thread From: Eli Zaretskii @ 2019-01-03 14:09 UTC (permalink / raw) To: Philipp Stephani; +Cc: phst, emacs-devel > 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-08-07 16:22 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-01-02 21:22 [PATCH] Ignore pending_signals when checking for quits Philipp Stephani 2019-01-03 14:03 ` Eli Zaretskii 2019-02-10 18:49 ` Philipp Stephani 2019-02-10 19:40 ` Eli Zaretskii 2019-02-10 19:46 ` Philipp Stephani 2019-02-10 20:15 ` Eli Zaretskii 2019-02-11 20:17 ` Philipp Stephani 2019-02-13 15:28 ` Eli Zaretskii 2019-02-24 21:51 ` Philipp Stephani 2019-04-19 19:06 ` Philipp Stephani 2019-04-19 20:15 ` Eli Zaretskii 2019-04-28 18:19 ` Philipp Stephani 2019-04-28 18:42 ` Eli Zaretskii 2019-08-07 16:22 ` Philipp Stephani 2019-01-03 14:09 ` Eli Zaretskii
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.