bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

[Paul Pogonyshev]

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 2.24.33,
cairo version 1.16.0) of 2022-07-26

Tested also with `emacs -Q', so this is not a problem of my configuration.

1. Open (a slow) remote connection, with TRAMP
2. Run M-x find-dired RET, arguments not important
3. Internal error "Wrong type: processp, nil" appears

Debugging suggests that it happens in `find-dired-with-command' when
calling `set-process-filter'. Apparently the asynchronously started shell
command started ~30 lines above counts as having finished (?) by then.

Paul

[-- Attachment #2: Type: text/html, Size: 721 bytes --]

bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

[Lars Ingebrigtsen]

Paul Pogonyshev <pogonyshev@gmail.com> writes:

> Tested also with `emacs -Q', so this is not a problem of my configuration.
>
> 1. Open (a slow) remote connection, with TRAMP
> 2. Run M-x find-dired RET, arguments not important
> 3. Internal error "Wrong type: processp, nil" appears
>
> Debugging suggests that it happens in `find-dired-with-command' when calling
> `set-process-filter'. Apparently the asynchronously started shell command started ~
> 30 lines above counts as having finished (?) by then.

The proper fix here would be to have a way to specify the
filter/sentinel in `shell-command', since that's the only way to really
guarantee that we'll be getting the output here.

But some testing seems to indicate that just moving the settings up
until right after the `shell-command' makes things work, so I've now
made that trivial fix in Emacs 29.  Does this fix this issue for you,
too?

bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

[Paul Pogonyshev]

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

Seems so. I guess with the way Elisp works it is even correct, because (as
I understand) Elisp has no way to notice that process has died if there are
no IO calls between `shell-process' and `set-process-*'. But it does feel
dirty. (Also that `(sit-for 1)'  a few lines above feels dirty.)

Paul

On Tue, 2 Aug 2022 at 12:35, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Paul Pogonyshev <pogonyshev@gmail.com> writes:
>
> > Tested also with `emacs -Q', so this is not a problem of my
> configuration.
> >
> > 1. Open (a slow) remote connection, with TRAMP
> > 2. Run M-x find-dired RET, arguments not important
> > 3. Internal error "Wrong type: processp, nil" appears
> >
> > Debugging suggests that it happens in `find-dired-with-command' when
> calling
> > `set-process-filter'. Apparently the asynchronously started shell
> command started ~
> > 30 lines above counts as having finished (?) by then.
>
> The proper fix here would be to have a way to specify the
> filter/sentinel in `shell-command', since that's the only way to really
> guarantee that we'll be getting the output here.
>
> But some testing seems to indicate that just moving the settings up
> until right after the `shell-command' makes things work, so I've now
> made that trivial fix in Emacs 29.  Does this fix this issue for you,
> too?
>
>
>

[-- Attachment #2: Type: text/html, Size: 1827 bytes --]

bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

[Lars Ingebrigtsen]

Paul Pogonyshev <pogonyshev@gmail.com> writes:

> Seems so. I guess with the way Elisp works it is even correct, because
> (as I understand) Elisp has no way to notice that process has died if
> there are no IO calls between `shell-process' and `set-process-*'.

Yes, but I'm not quite sure that's actually the case in all
circumstances (especially when Tramp is involved)...

> But it does feel dirty. (Also that `(sit-for 1)' a few lines above
> feels dirty.)

Yeah, much of the code in find-dired.el looks pretty fragile.

But I guess this works now, so I'm closing this bug report.

bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

[Paul Pogonyshev]

[-- Attachment #1: Type: text/plain, Size: 1700 bytes --]

Actually no, it doesn't help in all cases. It got better, but sometimes
still dies with the same error:

Debugger entered--Lisp error: (wrong-type-argument processp nil)
  process-mark(nil)
  (move-marker (process-mark proc) (point) (current-buffer))
  (let ((proc (get-buffer-process (current-buffer)))) (message "@ %S %S"
(current-buffer) proc) (move-marker (process-mark proc) (point)
(current-buffer)) (set-process-filter proc #'find-dired-filter)
(set-process-sentinel proc #'find-dired-sentinel))
  ...

For debugging I also added this line:

     (shell-command (concat command "&") (current-buffer))
     (let ((proc (get-buffer-process (current-buffer))))
 +     (message "@ %S %S" (current-buffer) proc)
       ;; Initialize the process marker; it is used by the filter.
       (move-marker (process-mark proc) (point) (current-buffer))  ;; <--
dies here

Here is the resut in buffer *Messages*:

@ #<buffer *Find*> nil

So, the process can be nil immediately after `shell-command' returns too.

Paul


On Tue, 2 Aug 2022 at 13:28, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Paul Pogonyshev <pogonyshev@gmail.com> writes:
>
> > Seems so. I guess with the way Elisp works it is even correct, because
> > (as I understand) Elisp has no way to notice that process has died if
> > there are no IO calls between `shell-process' and `set-process-*'.
>
> Yes, but I'm not quite sure that's actually the case in all
> circumstances (especially when Tramp is involved)...
>
> > But it does feel dirty. (Also that `(sit-for 1)' a few lines above
> > feels dirty.)
>
> Yeah, much of the code in find-dired.el looks pretty fragile.
>
> But I guess this works now, so I'm closing this bug report.
>
>

[-- Attachment #2: Type: text/html, Size: 2458 bytes --]

bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

[Lars Ingebrigtsen]

Paul Pogonyshev <pogonyshev@gmail.com> writes:

> Actually no, it doesn't help in all cases. It got better, but sometimes still dies with the
> same error:
>
> Debugger entered--Lisp error: (wrong-type-argument processp nil)
>   process-mark(nil)
>   (move-marker (process-mark proc) (point) (current-buffer))

Yeah, I suspected that this was a too brittle solution.

The real solution would be to use `make-process' here so that we can
specify the filter/sentinel immediately, but I'm not sure how that will
interfere with Tramp?

Anyway, I'm reopening this bug report.

bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

[Paul Pogonyshev]

[-- Attachment #1: Type: text/plain, Size: 2210 bytes --]

For what it's worth, I use `grep-find' a lot over Tramp and have never had
problems with it. (Other than when it finds a match in something like a 500
KB minified JS, but this is not Tramp-specific; Emacs in general sucks with
very long lines in text files.) It works over compilation infrastructure
apparently. Compilation itself also works fine for me.

Paul

On Wed, 10 Aug 2022 at 15:26, Paul Pogonyshev <pogonyshev@gmail.com> wrote:

> Actually no, it doesn't help in all cases. It got better, but sometimes
> still dies with the same error:
>
> Debugger entered--Lisp error: (wrong-type-argument processp nil)
>   process-mark(nil)
>   (move-marker (process-mark proc) (point) (current-buffer))
>   (let ((proc (get-buffer-process (current-buffer)))) (message "@ %S %S"
> (current-buffer) proc) (move-marker (process-mark proc) (point)
> (current-buffer)) (set-process-filter proc #'find-dired-filter)
> (set-process-sentinel proc #'find-dired-sentinel))
>   ...
>
> For debugging I also added this line:
>
>      (shell-command (concat command "&") (current-buffer))
>      (let ((proc (get-buffer-process (current-buffer))))
>  +     (message "@ %S %S" (current-buffer) proc)
>        ;; Initialize the process marker; it is used by the filter.
>        (move-marker (process-mark proc) (point) (current-buffer))  ;; <--
> dies here
>
> Here is the resut in buffer *Messages*:
>
> @ #<buffer *Find*> nil
>
> So, the process can be nil immediately after `shell-command' returns too.
>
> Paul
>
>
> On Tue, 2 Aug 2022 at 13:28, Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
>> Paul Pogonyshev <pogonyshev@gmail.com> writes:
>>
>> > Seems so. I guess with the way Elisp works it is even correct, because
>> > (as I understand) Elisp has no way to notice that process has died if
>> > there are no IO calls between `shell-process' and `set-process-*'.
>>
>> Yes, but I'm not quite sure that's actually the case in all
>> circumstances (especially when Tramp is involved)...
>>
>> > But it does feel dirty. (Also that `(sit-for 1)' a few lines above
>> > feels dirty.)
>>
>> Yeah, much of the code in find-dired.el looks pretty fragile.
>>
>> But I guess this works now, so I'm closing this bug report.
>>
>>

[-- Attachment #2: Type: text/html, Size: 3225 bytes --]

bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

[Michael Albinus]

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

> The real solution would be to use `make-process' here so that we can
> specify the filter/sentinel immediately, but I'm not sure how that will
> interfere with Tramp?

Tramp supports filters and sentinels in make-process.

Best regards, Michael.

bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

[Lars Ingebrigtsen]

Michael Albinus <michael.albinus@gmx.de> writes:

> Tramp supports filters and sentinels in make-process.

Great; then I think the rewrite to use make-process should be pretty
straightforward.

bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

[Liu Hui]

[-- Attachment #1: Type: text/plain, Size: 94 bytes --]


Hi,

I create a patch according to the discussion and find-dired works well
now.

Best,
Hui


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Set-the-filter-sentinel-of-find-process-reliably-in-.patch --]
[-- Type: text/x-diff, Size: 1438 bytes --]

From 8fbd85c80909b4cb560ec77cf84374e0a16b41df Mon Sep 17 00:00:00 2001
From: Liu Hui <liuhui1610@gmail.com>
Date: Thu, 1 Dec 2022 15:03:06 +0800
Subject: [PATCH] Set the filter/sentinel of find process reliably in
 find-dired

* lisp/find-dired.el (find-dired-with-command): Use `make-process'
rather than `shell-command' to start a find process.
---
 lisp/find-dired.el | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lisp/find-dired.el b/lisp/find-dired.el
index be3d106912..3d1ece2003 100644
--- a/lisp/find-dired.el
+++ b/lisp/find-dired.el
@@ -240,12 +240,13 @@ find-dired-with-command
     (erase-buffer)
     (setq default-directory dir)
     ;; Start the find process.
-    (shell-command (concat command "&") (current-buffer))
-    (let ((proc (get-buffer-process (current-buffer))))
-      ;; Initialize the process marker; it is used by the filter.
-      (move-marker (process-mark proc) (point) (current-buffer))
-      (set-process-filter proc #'find-dired-filter)
-      (set-process-sentinel proc #'find-dired-sentinel))
+    (make-process
+     :name "find-dired"
+     :buffer (current-buffer)
+     :command (split-string-shell-command command)
+     :filter #'find-dired-filter
+     :sentinel #'find-dired-sentinel
+     :file-handler t)
     (dired-mode dir (cdr find-ls-option))
     (let ((map (make-sparse-keymap)))
       (set-keymap-parent map (current-local-map))
-- 
2.25.1

bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

[Michael Albinus]

Liu Hui <liuhui1610@gmail.com> writes:

> Hi,

Hi Hui,

> I create a patch according to the discussion and find-dired works well
> now.

Thanks, it looks good to me. I'm not sure whether it will always work,
because the embedding shell, added by `shell-command', is not here. But
likely, it doesn't matter.

Paul, could you pls test this patch whether it works for you?

> Best,
> Hui

Best regards, Michael.

bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

[Paul Pogonyshev]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

I can only tell that it doesn't fail every time. The previous
implementation didn't fail constantly, only from time to time, so it's hard
to tell if the new is better.

> it looks good to me

So this is probably a better indicator.

Paul

On Thu, 1 Dec 2022 at 10:21, Michael Albinus <michael.albinus@gmx.de> wrote:

> Liu Hui <liuhui1610@gmail.com> writes:
>
> > Hi,
>
> Hi Hui,
>
> > I create a patch according to the discussion and find-dired works well
> > now.
>
> Thanks, it looks good to me. I'm not sure whether it will always work,
> because the embedding shell, added by `shell-command', is not here. But
> likely, it doesn't matter.
>
> Paul, could you pls test this patch whether it works for you?
>
> > Best,
> > Hui
>
> Best regards, Michael.
>

[-- Attachment #2: Type: text/html, Size: 1285 bytes --]

bug#56865: M-x find-dired fails with "Wrong type: processp, nil"

[Stefan Kangas]

Michael Albinus <michael.albinus@gmx.de> writes:

> Liu Hui <liuhui1610@gmail.com> writes:
>
>> Hi,
>
> Hi Hui,
>
>> I create a patch according to the discussion and find-dired works well
>> now.
>
> Thanks, it looks good to me. I'm not sure whether it will always work,
> because the embedding shell, added by `shell-command', is not here. But
> likely, it doesn't matter.
>
> Paul, could you pls test this patch whether it works for you?
>
>> Best,
>> Hui
>
> Best regards, Michael.

This patch doesn't apply any more after the below commit.

commit 7bbd7cae0748958a623f23637b95a6fc9debb8b7
Author: Michael Albinus <michael.albinus@gmx.de>
Date:   Mon Jul 31 17:49:21 2023 +0200

    Fix find-dired-with-command for remote directories

    * lisp/find-dired.el (find-dired-with-command):
    Use `start-file-process-shell-command'.  (Bug#64897)