bug#12145: 24.1.50; enable-remote-dir-locals breaks find-name-dired in TRAMP buffers

bug#12145: 24.1.50; enable-remote-dir-locals breaks find-name-dired in TRAMP buffers

[Matt McClure]

When `enable-remote-dir-locals` is `t`, `find-name-dired` in TRAMP buffers gives

    ...
    error in process filter: Wrong type argument: markerp, nil
    error in process filter: accept-process-output: Wrong type
argument: markerp, nil
    error in process filter: Wrong type argument: markerp, nil
    nil: exited abnormally with code 1.
    find-dired: Wrong type argument: processp, nil

To reproduce on Mac OS X:

    open -n -a Emacs --args -Q --no-site-file --eval '(progn (setq
enable-remote-dir-locals t) (find-name-dired "/user@host:/etc"
"*.conf"))'

I'm not able to reproduce the defect with the similar invocation:

    open -n -a Emacs --args -Q --no-site-file --eval '(progn
(find-name-dired "/user@host:/etc" "*.conf"))'


In GNU Emacs 24.1.50.1 (x86_64-apple-darwin, NS apple-appkit-1038.36)
 of 2012-07-14 on bob.porkrind.org
Bzr revision: 109087 cyd@gnu.org-20120714053223-jxkxt958pqg8tisb
Windowing system distributor `Apple', version 10.3.1138
Configured using:
 `configure '--host=x86_64-apple-darwin' '--build=i686-apple-darwin'
 '--with-ns' 'build_alias=i686-apple-darwin'
 'host_alias=x86_64-apple-darwin' 'CC=gcc -mmacosx-version-min=10.5''

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Fundamental

Minor modes in effect:
  shell-dirtrack-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
C-x C-b C-n C-x o C-n C-n <return> C-x 1 <escape> v
<escape> v C-a <escape> > C-p C-p C-p C-p C-p C-p C-p
C-SPC C-n C-n C-n C-n C-n C-n C-g C-x h <escape> w
<escape> x e <backspace> r e p o r <tab> <return>

Recent messages:
error in process filter: Wrong type argument: markerp, nil
nil: exited abnormally with code 1.
find-dired: Wrong type argument: processp, nil
Mark set [2 times]
Quit
Mark set [2 times]
Saved text until ": processp, nil
Mark set [2 times]
Quit
"

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rfc822 mml easymenu mml-sec
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mail-utils mule-util
tramp-cache tramp-sh tramp tramp-compat auth-source eieio byte-opt
bytecomp byte-compile cconv macroexp gnus-util mm-util mail-prsvr
password-cache tramp-loaddefs shell pcomplete comint ansi-color ring
format-spec advice help-fns advice-preload find-dired dired time-date
tooltip ediff-hook vc-hooks lisp-float-type mwheel ns-win tool-bar dnd
fontset image regexp-opt fringe tabulated-list newcomment lisp-mode
register page menu-bar rfn-eshadow timer select scroll-bar mouse
jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer loaddefs button faces cus-face files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process ns multi-tty
emacs)


-- 
Matt McClure
http://www.matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure

bug#12145: 24.1.50; enable-remote-dir-locals breaks find-name-dired in TRAMP buffers

[Michael Albinus]

Matt McClure <matthewlmcclure@gmail.com> writes:

> When `enable-remote-dir-locals` is `t`, `find-name-dired` in TRAMP buffers gives
>
>     error in process filter: Wrong type argument: markerp, nil

I can reproduce the bug locally. It happens in `comint-output-filter',
which is enabled by `tramp-handle-shell-command'.

> I'm not able to reproduce the defect with the similar invocation:
>
>     open -n -a Emacs --args -Q --no-site-file --eval '(progn
> (find-name-dired "/user@host:/etc" "*.conf"))'

Same here. It is a side-effect of an additional call to `file-exists-p'
for the directory local file; it is not related to directory local
settings as such.

Two Tramp processes do interact badly, when reading processes' output in
parallel.

I will continue to debug, it isn't such obvious what happens.

Best regards, Michael.

bug#12145: 24.1.50; enable-remote-dir-locals breaks find-name-dired in TRAMP buffers

[Michael Albinus]

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

> Matt McClure <matthewlmcclure@gmail.com> writes:
>
>> When `enable-remote-dir-locals` is `t`, `find-name-dired` in TRAMP
>> buffers gives
>>
>>     error in process filter: Wrong type argument: markerp, nil
>
> I can reproduce the bug locally. It happens in `comint-output-filter',
> which is enabled by `tramp-handle-shell-command'.
>
> Two Tramp processes do interact badly, when reading processes' output in
> parallel.
>
> I will continue to debug, it isn't such obvious what happens.

I've committed a fix for that. You might test it.

Best regards, Michael.

bug#12145: 24.1.50; enable-remote-dir-locals breaks find-name-dired in TRAMP buffers

[Matt McClure]

Hi Michael,

On Mon, Aug 20, 2012 at 10:07 AM, Michael Albinus
<michael.albinus@gmx.de> wrote:
> I've committed a fix for that. You might test it.

The fix works for me. Thanks!

-- 
Matt McClure
http://www.matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure

bug#12145: 24.1.50; enable-remote-dir-locals breaks find-name-dired in TRAMP buffers

[Michael Albinus]

Version: 24.3

Matt McClure <matthewlmcclure@gmail.com> writes:

> Hi Michael,

Hi Matt,

> On Mon, Aug 20, 2012 at 10:07 AM, Michael Albinus
> <michael.albinus@gmx.de> wrote:
>> I've committed a fix for that. You might test it.
>
> The fix works for me. Thanks!

Thanks for testing. I'm closing the bug.

Best regards, Michael.

bug#12145: Fix causes problems with other processes

[Thomas Koch]

The fix for this bug seems to cause a problem with "concurrent" processes over SSH with ControlMaster, see https://debbugs.gnu.org/61350

For testing I've removed the JUST-THIS-ONE arguments from all accept-process-output calls in Tramp. Afterwards the bug can still be reproduced in tramp 2.6.0 as described above.

At this point I wanted to paste the backtrace of the issue. However I found myself unable to reproduce the issue a second time. Only after I started eglot, I could trigger actually two different backtraces, depending on the starting folder for `find-name-dired` or the pattern.


Debugger entered--Lisp error: (wrong-type-argument processp nil)
  set-process-filter(nil find-dired-filter)
  find-dired("/blacky:/home/thk/git/yacy_search_server/source/ne..." "-name \\*.java")
  find-name-dired("/blacky:/home/thk/git/yacy_search_server/source/ne..." "*.java")
  funcall-interactively(find-name-dired "/blacky:/home/thk/git/yacy_search_server/source/ne..." "*.java")
  call-interactively(find-name-dired record nil)
  command-execute(find-name-dired record)
  execute-extended-command(nil "find-name-dired" nil)
  funcall-interactively(execute-extended-command nil "find-name-dired" nil)
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)


Debugger entered--Lisp error: (wrong-type-argument "markerp nil")
  signal(wrong-type-argument ("markerp nil"))
  tramp-error((tramp-file-name #("scp" 0 3 (tramp-default t)) nil nil "blacky" nil "/home/thk/git/yacy_search_server/.dir-locals.el" nil) wrong-type-argument "markerp nil")
  tramp-signal-hook-function(wrong-type-argument (markerp nil))
  comint-output-filter(#<process *Find*> "    74785      4 -rw-r--r--   1 thk      thk      ...")
  accept-process-output(#<process *tramp/scp blacky*> nil nil t)
  tramp-accept-process-output(#<process *tramp/scp blacky*>)
  tramp-wait-for-regexp(#<process *tramp/scp blacky*> nil "\\(?:^\\|\0\\)\\(?:[^\n#$]*///ad07405faf7fa5751ee9747c48...")
  tramp-wait-for-output(#<process *tramp/scp blacky*>)
  tramp-send-command((tramp-file-name #("scp" 0 3 (tramp-default t)) nil nil "blacky" nil "/home/thk/git/yacy_search_server/.dir-locals.el" nil) "test -r /home/thk/git/yacy_search_server/.dir-loca...")
  tramp-send-command-and-check((tramp-file-name #("scp" 0 3 (tramp-default t)) nil nil "blacky" nil "/home/thk/git/yacy_search_server/.dir-locals.el" nil) "test -r /home/thk/git/yacy_search_server/.dir-loca...")
  tramp-run-test("-r" "/blacky:/home/thk/git/yacy_search_server/.dir-loca...")
  tramp-sh-handle-file-readable-p("/blacky:/home/thk/git/yacy_search_server/.dir-loca...")
  apply(tramp-sh-handle-file-readable-p "/blacky:/home/thk/git/yacy_search_server/.dir-loca...")
  tramp-sh-file-name-handler(file-readable-p "/blacky:/home/thk/git/yacy_search_server/.dir-loca...")
  apply(tramp-sh-file-name-handler file-readable-p "/blacky:/home/thk/git/yacy_search_server/.dir-loca...")
  tramp-file-name-handler(file-readable-p "/blacky:/home/thk/git/yacy_search_server/.dir-loca...")
  file-readable-p("/blacky:/home/thk/git/yacy_search_server/.dir-loca...")
  dir-locals--all-files("/blacky:/home/thk/git/yacy_search_server/")
  locate-dominating-file("/blacky:/home/thk/git/yacy_search_server/" dir-locals--all-files)
  dir-locals-find-file("/blacky:/home/thk/git/yacy_search_server/")
  hack-dir-local-variables()
  hack-dir-local-variables-non-file-buffer()
  dired-mode("/blacky:/home/thk/git/yacy_search_server/" "-dilsb")
  find-dired("/blacky:/home/thk/git/yacy_search_server/" "-name \\*.bat")
  find-name-dired("/blacky:/home/thk/git/yacy_search_server/" "*.bat")
  funcall-interactively(find-name-dired "/blacky:/home/thk/git/yacy_search_server/" "*.bat")
  call-interactively(find-name-dired record nil)
  command-execute(find-name-dired record)
  execute-extended-command(nil "find-name-dired" nil)
  funcall-interactively(execute-extended-command nil "find-name-dired" nil)
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)


I'd like to try to understand this bug better and to find a fix that does not set JUST-THIS-ONE to t. Any help is appreciated.

bug#12145: Fix causes problems with other processes

[Thomas Koch]

The problem identified by the first backtrace still exists even with the patch for this bug from 2012. It happens, when find-name-dired on a remote system returns very fast (or maybe when the output is very short?), e.g. when called on an empty directory.

The function `find-dired` starts a background shell process:

    (shell-command (concat args "&") (current-buffer))

A few lines later, the function asks for the process and uses it without any error checking:

    (let ((proc (get-buffer-process (current-buffer))))
      (set-process-filter proc (function find-dired-filter))

I assume, that get-buffer-process returns nil when the process has already terminated.

I don't have an idea about the second backtrace yet. But given that the code of find-name-dired seems problematic, this might be a motivation to not search for a fix in tramp but in find-name-dired (and called functions).

bug#12145: Fix causes problems with other processes

[Michael Albinus]

Thomas Koch <thomas@koch.ro> writes:

Hi Thomas,

> The problem identified by the first backtrace still exists even with the patch for this bug from 2012. It happens, when find-name-dired on a remote system returns very fast (or maybe when the output is very short?), e.g. when called on an empty directory.
>
> The function `find-dired` starts a background shell process:
>
>     (shell-command (concat args "&") (current-buffer))
>
> A few lines later, the function asks for the process and uses it without any error checking:
>
>     (let ((proc (get-buffer-process (current-buffer))))
>       (set-process-filter proc (function find-dired-filter))
>
> I assume, that get-buffer-process returns nil when the process has already terminated.

Good analysis, thanks. I'll play with this, hopefully I can reproduce.

On a first glance (w/o testing), something like this could work:

(let (proc)
  (while (not (process-alive-p (setq proc (get-buffer-process (current-buffer))))))
  (set-process-filter proc (function find-dired-filter))
  ...)

> I don't have an idea about the second backtrace yet. But given that the code of find-name-dired seems problematic, this might be a motivation to not search for a fix in tramp but in find-name-dired (and called functions).

I'll check.

Best regards, Michael.

bug#12145: Fix causes problems with other processes

[Thomas Koch]

I've now also digged a bit into the second, longer backtrace with (wrong-type-argument "markerp nil"):

- With (message) debugging I fount that the last line of this snippet in `comint-output-filter` is the one that causes the error:

	  ;; We temporarily remove any buffer narrowing, in case the
	  ;; process mark is outside of the restriction
	  (save-restriction
	    (widen)

	    (goto-char (process-mark process))
	    (set-marker comint-last-output-start (point))

- comint-last-output start gets initialized in `define-derived-mode comint-mode`:

  (setq-local comint-last-output-start (make-marker))

-> Either, a) `comint-output-filter` gets called before `comint-mode`
-> or b) `comint-last-output-start` gets reset after its initialization.

These two lines from `find-dired`:

    (shell-command (concat args "&") (current-buffer))
    ;; The next statement will bomb in classic dired (no optional arg allowed)
    (dired-mode dir (cdr find-ls-option))

1. `shell-command` switches (current-buffer) to `shell-mode` which is derived from `comint-mode`.
2. `comint-mode` initializes `comint-last-output-start`.
3. `shell-command` sets `comint-output-filter` for the process it started
4. `find-dired` calls `dired-mode`
5. `dired-mode` calls (kill-all-local-variables) which probably sets `comint-last-output-start` to nil?

It is however not too important whether the above analysis is right, given the following comment in shell-command:

"""In Elisp, you will often be better served by calling call-process or
   start-process directly, since they offer more control and do not
   impose the use of a shell (with its need to quote arguments)."""

I believe, find-dired should be changed to not call shell-command anymore.

I rest my case.