diff options
| author | Philipp Stephani | 2019-01-02 22:04:56 +0100 |
|---|---|---|
| committer | Philipp Stephani | 2019-02-24 22:43:07 +0100 |
| commit | 72ec233f2a1b8a6a9574e61588d0467caf41755c (patch) | |
| tree | 725add4413feb9cb7789576294099096e63d3044 | |
| parent | 5653b76d0bacf1edfc3d962c0bb991344cd80f6f (diff) | |
| download | emacs-72ec233f2a1b8a6a9574e61588d0467caf41755c.tar.gz emacs-72ec233f2a1b8a6a9574e61588d0467caf41755c.zip | |
Ignore pending_signals when checking for quits.
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.
| -rw-r--r-- | configure.ac | 2 | ||||
| -rw-r--r-- | doc/lispref/internals.texi | 22 | ||||
| -rw-r--r-- | etc/NEWS | 3 | ||||
| -rw-r--r-- | src/emacs-module.c | 15 | ||||
| -rw-r--r-- | src/emacs-module.h.in | 21 | ||||
| -rw-r--r-- | src/eval.c | 5 | ||||
| -rw-r--r-- | src/module-env-27.h | 4 | ||||
| -rw-r--r-- | test/data/emacs-module/mod-test.c | 69 | ||||
| -rw-r--r-- | test/src/emacs-module-tests.el | 20 |
9 files changed, 151 insertions, 10 deletions
diff --git a/configure.ac b/configure.ac index c26eb6d1e89..110ea2909a9 100644 --- a/configure.ac +++ b/configure.ac | |||
| @@ -3689,8 +3689,10 @@ AC_SUBST(MODULES_SUFFIX) | |||
| 3689 | AC_CONFIG_FILES([src/emacs-module.h]) | 3689 | AC_CONFIG_FILES([src/emacs-module.h]) |
| 3690 | AC_SUBST_FILE([module_env_snippet_25]) | 3690 | AC_SUBST_FILE([module_env_snippet_25]) |
| 3691 | AC_SUBST_FILE([module_env_snippet_26]) | 3691 | AC_SUBST_FILE([module_env_snippet_26]) |
| 3692 | AC_SUBST_FILE([module_env_snippet_27]) | ||
| 3692 | module_env_snippet_25="$srcdir/src/module-env-25.h" | 3693 | module_env_snippet_25="$srcdir/src/module-env-25.h" |
| 3693 | module_env_snippet_26="$srcdir/src/module-env-26.h" | 3694 | module_env_snippet_26="$srcdir/src/module-env-26.h" |
| 3695 | module_env_snippet_27="$srcdir/src/module-env-27.h" | ||
| 3694 | 3696 | ||
| 3695 | ### Use -lpng if available, unless '--with-png=no'. | 3697 | ### Use -lpng if available, unless '--with-png=no'. |
| 3696 | HAVE_PNG=no | 3698 | HAVE_PNG=no |
diff --git a/doc/lispref/internals.texi b/doc/lispref/internals.texi index 3fbff266add..56465126f41 100644 --- a/doc/lispref/internals.texi +++ b/doc/lispref/internals.texi | |||
| @@ -1623,7 +1623,27 @@ purpose. | |||
| 1623 | @deftypefn Function bool should_quit (emacs_env *@var{env}) | 1623 | @deftypefn Function bool should_quit (emacs_env *@var{env}) |
| 1624 | This function returns @code{true} if the user wants to quit. In that | 1624 | This function returns @code{true} if the user wants to quit. In that |
| 1625 | case, we recommend that your module function aborts any on-going | 1625 | case, we recommend that your module function aborts any on-going |
| 1626 | processing and returns as soon as possible. | 1626 | processing and returns as soon as possible. In most cases, use |
| 1627 | @code{process_input} instead. | ||
| 1628 | @end deftypefn | ||
| 1629 | |||
| 1630 | To process input events in addition to checking whether the user wants | ||
| 1631 | to quit, use the following function, which is available since Emacs | ||
| 1632 | 27.1. | ||
| 1633 | |||
| 1634 | @anchor{process_input} | ||
| 1635 | @deftypefn Function enum emacs_process_input_result process_input (emacs_env *@var{env}) | ||
| 1636 | This function processes pending input events. It returns | ||
| 1637 | @code{emacs_process_input_quit} if the user wants to quit or an error | ||
| 1638 | occurred while processing signals. In that case, we recommend that | ||
| 1639 | your module function aborts any on-going processing and returns as | ||
| 1640 | soon as possible. If the module code may continue running, | ||
| 1641 | @code{process_input} returns @code{emacs_process_input_continue}. The | ||
| 1642 | return value is @code{emacs_process_input_continue} if and only if | ||
| 1643 | there is no pending nonlocal exit in @code{env}. If the module | ||
| 1644 | continues after calling @code{process_input}, global state such as | ||
| 1645 | variable values and buffer content may have been modified in arbitrary | ||
| 1646 | ways. | ||
| 1627 | @end deftypefn | 1647 | @end deftypefn |
| 1628 | 1648 | ||
| 1629 | @node Module Nonlocal | 1649 | @node Module Nonlocal |
| @@ -1614,6 +1614,9 @@ given frame supports resizing. | |||
| 1614 | This is currently supported on GNUish hosts and on modern versions of | 1614 | This is currently supported on GNUish hosts and on modern versions of |
| 1615 | MS-Windows. | 1615 | MS-Windows. |
| 1616 | 1616 | ||
| 1617 | ** New module environment function 'process_input' to process user | ||
| 1618 | input while module code is running. | ||
| 1619 | |||
| 1617 | 1620 | ||
| 1618 | * Changes in Emacs 27.1 on Non-Free Operating Systems | 1621 | * Changes in Emacs 27.1 on Non-Free Operating Systems |
| 1619 | 1622 | ||
diff --git a/src/emacs-module.c b/src/emacs-module.c index cbab0234201..b70d6cea812 100644 --- a/src/emacs-module.c +++ b/src/emacs-module.c | |||
| @@ -671,13 +671,21 @@ module_vec_size (emacs_env *env, emacs_value vec) | |||
| 671 | return ASIZE (lvec); | 671 | return ASIZE (lvec); |
| 672 | } | 672 | } |
| 673 | 673 | ||
| 674 | /* This function should return true if and only if maybe_quit would do | 674 | /* This function should return true if and only if maybe_quit would |
| 675 | anything. */ | 675 | quit. */ |
| 676 | static bool | 676 | static bool |
| 677 | module_should_quit (emacs_env *env) | 677 | module_should_quit (emacs_env *env) |
| 678 | { | 678 | { |
| 679 | MODULE_FUNCTION_BEGIN_NO_CATCH (false); | 679 | MODULE_FUNCTION_BEGIN_NO_CATCH (false); |
| 680 | return (! NILP (Vquit_flag) && NILP (Vinhibit_quit)) || pending_signals; | 680 | return QUITP; |
| 681 | } | ||
| 682 | |||
| 683 | static enum emacs_process_input_result | ||
| 684 | module_process_input (emacs_env *env) | ||
| 685 | { | ||
| 686 | MODULE_FUNCTION_BEGIN (emacs_process_input_quit); | ||
| 687 | maybe_quit (); | ||
| 688 | return emacs_process_input_continue; | ||
| 681 | } | 689 | } |
| 682 | 690 | ||
| 683 | 691 | ||
| @@ -1082,6 +1090,7 @@ initialize_environment (emacs_env *env, struct emacs_env_private *priv) | |||
| 1082 | env->vec_get = module_vec_get; | 1090 | env->vec_get = module_vec_get; |
| 1083 | env->vec_size = module_vec_size; | 1091 | env->vec_size = module_vec_size; |
| 1084 | env->should_quit = module_should_quit; | 1092 | env->should_quit = module_should_quit; |
| 1093 | env->process_input = module_process_input; | ||
| 1085 | Vmodule_environments = Fcons (make_mint_ptr (env), Vmodule_environments); | 1094 | Vmodule_environments = Fcons (make_mint_ptr (env), Vmodule_environments); |
| 1086 | return env; | 1095 | return env; |
| 1087 | } | 1096 | } |
diff --git a/src/emacs-module.h.in b/src/emacs-module.h.in index 4c5286f6257..009d1583fef 100644 --- a/src/emacs-module.h.in +++ b/src/emacs-module.h.in | |||
| @@ -47,7 +47,7 @@ extern "C" { | |||
| 47 | #endif | 47 | #endif |
| 48 | 48 | ||
| 49 | /* Current environment. */ | 49 | /* Current environment. */ |
| 50 | typedef struct emacs_env_26 emacs_env; | 50 | typedef struct emacs_env_27 emacs_env; |
| 51 | 51 | ||
| 52 | /* Opaque pointer representing an Emacs Lisp value. | 52 | /* Opaque pointer representing an Emacs Lisp value. |
| 53 | BEWARE: Do not assume NULL is a valid value! */ | 53 | BEWARE: Do not assume NULL is a valid value! */ |
| @@ -83,6 +83,16 @@ enum emacs_funcall_exit | |||
| 83 | emacs_funcall_exit_throw = 2 | 83 | emacs_funcall_exit_throw = 2 |
| 84 | }; | 84 | }; |
| 85 | 85 | ||
| 86 | /* Possible return values for emacs_env.process_input. */ | ||
| 87 | enum emacs_process_input_result | ||
| 88 | { | ||
| 89 | /* Module code may continue */ | ||
| 90 | emacs_process_input_continue = 0, | ||
| 91 | |||
| 92 | /* Module code should return control to Emacs as soon as possible. */ | ||
| 93 | emacs_process_input_quit = 1 | ||
| 94 | }; | ||
| 95 | |||
| 86 | struct emacs_env_25 | 96 | struct emacs_env_25 |
| 87 | { | 97 | { |
| 88 | @module_env_snippet_25@ | 98 | @module_env_snippet_25@ |
| @@ -95,6 +105,15 @@ struct emacs_env_26 | |||
| 95 | @module_env_snippet_26@ | 105 | @module_env_snippet_26@ |
| 96 | }; | 106 | }; |
| 97 | 107 | ||
| 108 | struct emacs_env_27 | ||
| 109 | { | ||
| 110 | @module_env_snippet_25@ | ||
| 111 | |||
| 112 | @module_env_snippet_26@ | ||
| 113 | |||
| 114 | @module_env_snippet_27@ | ||
| 115 | }; | ||
| 116 | |||
| 98 | /* Every module should define a function as follows. */ | 117 | /* Every module should define a function as follows. */ |
| 99 | extern int emacs_module_init (struct emacs_runtime *ert) | 118 | extern int emacs_module_init (struct emacs_runtime *ert) |
| 100 | EMACS_NOEXCEPT | 119 | EMACS_NOEXCEPT |
diff --git a/src/eval.c b/src/eval.c index b094fc2e663..b6cdfc911d0 100644 --- a/src/eval.c +++ b/src/eval.c | |||
| @@ -1575,10 +1575,7 @@ process_quit_flag (void) | |||
| 1575 | If quit-flag is set to `kill-emacs' the SIGINT handler has received | 1575 | If quit-flag is set to `kill-emacs' the SIGINT handler has received |
| 1576 | a request to exit Emacs when it is safe to do. | 1576 | a request to exit Emacs when it is safe to do. |
| 1577 | 1577 | ||
| 1578 | When not quitting, process any pending signals. | 1578 | When not quitting, process any pending signals. */ |
| 1579 | |||
| 1580 | If you change this function, also adapt module_should_quit in | ||
| 1581 | emacs-module.c. */ | ||
| 1582 | 1579 | ||
| 1583 | void | 1580 | void |
| 1584 | maybe_quit (void) | 1581 | maybe_quit (void) |
diff --git a/src/module-env-27.h b/src/module-env-27.h new file mode 100644 index 00000000000..b491b60fbbc --- /dev/null +++ b/src/module-env-27.h | |||
| @@ -0,0 +1,4 @@ | |||
| 1 | /* Processes pending input events and returns whether the module | ||
| 2 | function should quit. */ | ||
| 3 | enum emacs_process_input_result (*process_input) (emacs_env *env) | ||
| 4 | EMACS_ATTRIBUTE_NONNULL (1); | ||
diff --git a/test/data/emacs-module/mod-test.c b/test/data/emacs-module/mod-test.c index 98242e85baf..47ea159d0e7 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. | |||
| 17 | You should have received a copy of the GNU General Public License | 17 | You should have received a copy of the GNU General Public License |
| 18 | along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. */ | 18 | along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. */ |
| 19 | 19 | ||
| 20 | #include "config.h" | ||
| 21 | |||
| 20 | #include <assert.h> | 22 | #include <assert.h> |
| 23 | #include <errno.h> | ||
| 24 | #include <limits.h> | ||
| 21 | #include <stdio.h> | 25 | #include <stdio.h> |
| 22 | #include <stdlib.h> | 26 | #include <stdlib.h> |
| 23 | #include <limits.h> | 27 | #include <string.h> |
| 28 | #include <time.h> | ||
| 29 | |||
| 24 | #include <emacs-module.h> | 30 | #include <emacs-module.h> |
| 25 | 31 | ||
| 32 | #include "timespec.h" | ||
| 33 | |||
| 26 | int plugin_is_GPL_compatible; | 34 | int plugin_is_GPL_compatible; |
| 27 | 35 | ||
| 28 | #if INTPTR_MAX <= 0 | 36 | #if INTPTR_MAX <= 0 |
| @@ -299,6 +307,64 @@ Fmod_test_invalid_finalizer (emacs_env *env, ptrdiff_t nargs, emacs_value *args, | |||
| 299 | return env->funcall (env, env->intern (env, "garbage-collect"), 0, NULL); | 307 | return env->funcall (env, env->intern (env, "garbage-collect"), 0, NULL); |
| 300 | } | 308 | } |
| 301 | 309 | ||
| 310 | static void | ||
| 311 | signal_wrong_type_argument (emacs_env *env, const char *predicate, | ||
| 312 | emacs_value arg) | ||
| 313 | { | ||
| 314 | emacs_value symbol = env->intern (env, "wrong-type-argument"); | ||
| 315 | emacs_value elements[2] = {env->intern (env, predicate), arg}; | ||
| 316 | emacs_value data = env->funcall (env, env->intern (env, "list"), 2, elements); | ||
| 317 | env->non_local_exit_signal (env, symbol, data); | ||
| 318 | } | ||
| 319 | |||
| 320 | static void | ||
| 321 | signal_errno (emacs_env *env, const char *function) | ||
| 322 | { | ||
| 323 | const char *message = strerror (errno); | ||
| 324 | emacs_value message_value = env->make_string (env, message, strlen (message)); | ||
| 325 | emacs_value symbol = env->intern (env, "file-error"); | ||
| 326 | emacs_value elements[2] | ||
| 327 | = {env->make_string (env, function, strlen (function)), message_value}; | ||
| 328 | emacs_value data = env->funcall (env, env->intern (env, "list"), 2, elements); | ||
| 329 | env->non_local_exit_signal (env, symbol, data); | ||
| 330 | } | ||
| 331 | |||
| 332 | /* A long-running operation that occasionally calls `should_quit' or | ||
| 333 | `process_input'. */ | ||
| 334 | |||
| 335 | static emacs_value | ||
| 336 | Fmod_test_sleep_until (emacs_env *env, ptrdiff_t nargs, emacs_value *args, | ||
| 337 | void *data) | ||
| 338 | { | ||
| 339 | assert (nargs == 2); | ||
| 340 | const double until_seconds = env->extract_float (env, args[0]); | ||
| 341 | if (env->non_local_exit_check (env)) | ||
| 342 | return NULL; | ||
| 343 | if (until_seconds <= 0) | ||
| 344 | { | ||
| 345 | signal_wrong_type_argument (env, "cl-plusp", args[0]); | ||
| 346 | return NULL; | ||
| 347 | } | ||
| 348 | const bool process_input = env->is_not_nil (env, args[1]); | ||
| 349 | const struct timespec until = dtotimespec (until_seconds); | ||
| 350 | const struct timespec amount = make_timespec(0, 10000000); | ||
| 351 | while (true) | ||
| 352 | { | ||
| 353 | const struct timespec now = current_timespec (); | ||
| 354 | if (timespec_cmp (now, until) >= 0) | ||
| 355 | break; | ||
| 356 | if (nanosleep (&amount, NULL) && errno != EINTR) | ||
| 357 | { | ||
| 358 | signal_errno (env, "nanosleep"); | ||
| 359 | return NULL; | ||
| 360 | } | ||
| 361 | if ((process_input | ||
| 362 | && env->process_input (env) == emacs_process_input_quit) | ||
| 363 | || env->should_quit (env)) | ||
| 364 | return NULL; | ||
| 365 | } | ||
| 366 | return env->intern (env, "finished"); | ||
| 367 | } | ||
| 302 | 368 | ||
| 303 | /* Lisp utilities for easier readability (simple wrappers). */ | 369 | /* Lisp utilities for easier readability (simple wrappers). */ |
| 304 | 370 | ||
| @@ -367,6 +433,7 @@ emacs_module_init (struct emacs_runtime *ert) | |||
| 367 | DEFUN ("mod-test-invalid-load", Fmod_test_invalid_load, 0, 0, NULL, NULL); | 433 | DEFUN ("mod-test-invalid-load", Fmod_test_invalid_load, 0, 0, NULL, NULL); |
| 368 | DEFUN ("mod-test-invalid-finalizer", Fmod_test_invalid_finalizer, 0, 0, | 434 | DEFUN ("mod-test-invalid-finalizer", Fmod_test_invalid_finalizer, 0, 0, |
| 369 | NULL, NULL); | 435 | NULL, NULL); |
| 436 | DEFUN ("mod-test-sleep-until", Fmod_test_sleep_until, 2, 2, NULL, NULL); | ||
| 370 | 437 | ||
| 371 | #undef DEFUN | 438 | #undef DEFUN |
| 372 | 439 | ||
diff --git a/test/src/emacs-module-tests.el b/test/src/emacs-module-tests.el index e4593044ecd..e30980b5993 100644 --- a/test/src/emacs-module-tests.el +++ b/test/src/emacs-module-tests.el | |||
| @@ -289,4 +289,24 @@ Return A + B" | |||
| 289 | (should (member '(provide . mod-test) entries)) | 289 | (should (member '(provide . mod-test) entries)) |
| 290 | (should (member '(defun . mod-test-sum) entries)))) | 290 | (should (member '(defun . mod-test-sum) entries)))) |
| 291 | 291 | ||
| 292 | (ert-deftest mod-test-sleep-until () | ||
| 293 | "Check that `mod-test-sleep-until' either returns normally or quits. | ||
| 294 | Interactively, you can try hitting \\[keyboard-quit] to quit." | ||
| 295 | (dolist (arg '(nil t)) | ||
| 296 | ;; Guard against some caller setting `inhibit-quit'. | ||
| 297 | (with-local-quit | ||
| 298 | (condition-case nil | ||
| 299 | (should (eq (with-local-quit | ||
| 300 | ;; Because `inhibit-quit' is nil here, the next | ||
| 301 | ;; form either quits or returns `finished'. | ||
| 302 | (mod-test-sleep-until | ||
| 303 | ;; Interactively, run for 5 seconds to give the | ||
| 304 | ;; user time to quit. In batch mode, run only | ||
| 305 | ;; briefly since the user can't quit. | ||
| 306 | (float-time (time-add nil (if noninteractive 0.1 5))) | ||
| 307 | ;; should_quit or process_input | ||
| 308 | arg)) | ||
| 309 | 'finished)) | ||
| 310 | (quit))))) | ||
| 311 | |||
| 292 | ;;; emacs-module-tests.el ends here | 312 | ;;; emacs-module-tests.el ends here |