From: Noam Postavsky Date: Mon, 8 Apr 2019 21:57:22 +0000 (-0400) Subject: Let debugger handle process spawn errors on w32 (Bug#33016) X-Git-Tag: emacs-27.0.90~3250 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=9800df69cb7003bda1f2b98d6f11e89ba95afb9b;p=emacs.git Let debugger handle process spawn errors on w32 (Bug#33016) Since child_setup() is called between block_input()...unblock_input(), when an error is signaled the Lisp debugger is prevented from starting. Therefore, let the callers signal the error instead (which they already do for non-w32 platforms, just the error message needs an update). * src/callproc.c (child_setup) [WINDOWSNT]: Don't call report_file_error here. (call_process) [WINDOWNT]: * src/process.c (create_process) [WINDOWSNT]: Call report_file_errno here instead, after the unblock_input() call, same as for !WINDOWSNT. * src/lisp.h (CHILD_SETUP_ERROR_DESC): New preprocessor define. Flip the containing ifndef DOS_NT branches so that it's ifdef DOS_NT. * src/eval.c (when_entered_debugger): Remove. (syms_of_eval) : Define it as a Lisp integer variable instead. (maybe_call_debugger): Update comment. * test/src/process-tests.el (make-process-w32-debug-spawn-error): * test/src/callproc-tests.el (call-process-w32-debug-spawn-error): New tests. --- diff --git a/src/callproc.c b/src/callproc.c index a3d09609d7b..2cdf84d9a80 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -681,7 +681,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd, unblock_input (); if (pid < 0) - report_file_errno ("Doing vfork", Qnil, child_errno); + report_file_errno (CHILD_SETUP_ERROR_DESC, Qnil, child_errno); /* Close our file descriptors, except for callproc_fd[CALLPROC_PIPEREAD] since we will use that to read input from. */ @@ -1174,7 +1174,7 @@ exec_failed (char const *name, int err) executable directory by the parent. On GNUish hosts, either exec or return an error number. - On MS-Windows, either return a pid or signal an error. + On MS-Windows, either return a pid or return -1 and set errno. On MS-DOS, either return an exit status or signal an error. */ CHILD_SETUP_TYPE @@ -1319,9 +1319,6 @@ child_setup (int in, int out, int err, char **new_argv, bool set_pgrp, /* Spawn the child. (See w32proc.c:sys_spawnve). */ cpid = spawnve (_P_NOWAIT, new_argv[0], new_argv, env); reset_standard_handles (in, out, err, handles); - if (cpid == -1) - /* An error occurred while trying to spawn the process. */ - report_file_error ("Spawning child process", Qnil); return cpid; #else /* not WINDOWSNT */ diff --git a/src/eval.c b/src/eval.c index e9f118c5cb9..fa7b2d06031 100644 --- a/src/eval.c +++ b/src/eval.c @@ -52,15 +52,6 @@ Lisp_Object Vautoload_queue; is shutting down. */ Lisp_Object Vrun_hooks; -/* The value of num_nonmacro_input_events as of the last time we - started to enter the debugger. If we decide to enter the debugger - again when this is still equal to num_nonmacro_input_events, then we - know that the debugger itself has an error, and we should just - signal the error instead of entering an infinite loop of debugger - invocations. */ - -static intmax_t when_entered_debugger; - /* The function from which the last `signal' was called. Set in Fsignal. */ /* FIXME: We should probably get rid of this! */ @@ -1835,7 +1826,8 @@ maybe_call_debugger (Lisp_Object conditions, Lisp_Object sig, Lisp_Object data) ? debug_on_quit : wants_debugger (Vdebug_on_error, conditions)) && ! skip_debugger (conditions, combined_data) - /* RMS: What's this for? */ + /* See commentary on definition of + `internal-when-entered-debugger'. */ && when_entered_debugger < num_nonmacro_input_events) { call_debugger (list2 (Qerror, combined_data)); @@ -4170,6 +4162,18 @@ Note that `debug-on-error', `debug-on-quit' and friends still determine whether to handle the particular condition. */); Vdebug_on_signal = Qnil; + /* The value of num_nonmacro_input_events as of the last time we + started to enter the debugger. If we decide to enter the debugger + again when this is still equal to num_nonmacro_input_events, then we + know that the debugger itself has an error, and we should just + signal the error instead of entering an infinite loop of debugger + invocations. */ + DEFSYM (Qinternal_when_entered_debugger, "internal-when-entered-debugger"); + DEFVAR_INT ("internal-when-entered-debugger", when_entered_debugger, + doc: /* The number of keyboard events as of last time `debugger' was called. +Used to avoid infinite loops if the debugger itself has an error. +Don't set this unless you're sure that can't happen. */); + /* When lexical binding is being used, Vinternal_interpreter_environment is non-nil, and contains an alist of lexically-bound variable, or (t), indicating an empty diff --git a/src/lisp.h b/src/lisp.h index 681efc3b52b..2915944ffec 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -4480,11 +4480,14 @@ extern void syms_of_process (void); extern void setup_process_coding_systems (Lisp_Object); /* Defined in callproc.c. */ -#ifndef DOS_NT -# define CHILD_SETUP_TYPE _Noreturn void -#else +#ifdef DOS_NT # define CHILD_SETUP_TYPE int +# define CHILD_SETUP_ERROR_DESC "Spawning child process" +#else +# define CHILD_SETUP_TYPE _Noreturn void +# define CHILD_SETUP_ERROR_DESC "Doing vfork" #endif + extern CHILD_SETUP_TYPE child_setup (int, int, int, char **, bool, Lisp_Object); extern void init_callproc_1 (void); extern void init_callproc (void); diff --git a/src/process.c b/src/process.c index 6770a5ed884..0c440371628 100644 --- a/src/process.c +++ b/src/process.c @@ -2233,7 +2233,7 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) unblock_input (); if (pid < 0) - report_file_errno ("Doing vfork", Qnil, vfork_errno); + report_file_errno (CHILD_SETUP_ERROR_DESC, Qnil, vfork_errno); else { /* vfork succeeded. */ diff --git a/test/src/callproc-tests.el b/test/src/callproc-tests.el index 7b30a251cce..f351b6e2148 100644 --- a/test/src/callproc-tests.el +++ b/test/src/callproc-tests.el @@ -37,3 +37,25 @@ (split-string-and-unquote (buffer-string))) (should (equal initial-shell "nil")) (should-not (equal initial-shell shell)))) + +(ert-deftest call-process-w32-debug-spawn-error () + "Check that debugger runs on `call-process' failure (Bug#33016)." + (skip-unless (eq system-type 'windows-nt)) + (let* ((debug-on-error t) + (have-called-debugger nil) + (debugger (lambda (&rest _) + (setq have-called-debugger t) + ;; Allow entering the debugger later in the same + ;; test run, before going back to the command + ;; loop. + (setq internal-when-entered-debugger -1)))) + (should (eq :got-error ;; NOTE: `should-error' would inhibit debugger. + (condition-case-unless-debug () + ;; On Windows, "nul.FOO" act like an always-empty + ;; file for any FOO, in any directory. So this + ;; passes Emacs' test for the file's existence, + ;; and ensures we hit an error in the w32 process + ;; spawn code. + (call-process "c:/nul.exe") + (error :got-error)))) + (should have-called-debugger))) diff --git a/test/src/process-tests.el b/test/src/process-tests.el index 5dbf441e8c2..0bb7ebe50a8 100644 --- a/test/src/process-tests.el +++ b/test/src/process-tests.el @@ -215,6 +215,26 @@ (string-to-list "stdout\n") (string-to-list "stderr\n")))))) +(ert-deftest make-process-w32-debug-spawn-error () + "Check that debugger runs on `make-process' failure (Bug#33016)." + (skip-unless (eq system-type 'windows-nt)) + (let* ((debug-on-error t) + (have-called-debugger nil) + (debugger (lambda (&rest _) + (setq have-called-debugger t) + ;; Allow entering the debugger later in the same + ;; test run, before going back to the command + ;; loop. + (setq internal-when-entered-debugger -1)))) + (should (eq :got-error ;; NOTE: `should-error' would inhibit debugger. + (condition-case-unless-debug () + ;; Emacs doesn't search for absolute filenames, so + ;; the error will be hit in the w32 process spawn + ;; code. + (make-process :name "test" :command '("c:/No-Such-Command")) + (error :got-error)))) + (should have-called-debugger))) + (ert-deftest make-process/file-handler/found () "Check that the ‘:file-handler’ argument of ‘make-process’ works as expected if a file name handler is found."