Commit 9800df69 authored by Noam Postavsky's avatar Noam Postavsky

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) <internal-when-entered-debugger>: 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.
parent fc0f469f
Pipeline #1296 failed with stage
in 49 minutes and 13 seconds
......@@ -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 */
......
......@@ -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
......
......@@ -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);
......
......@@ -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. */
......
......@@ -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)))
......@@ -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."
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment