Race condition in eshell-gather-process-output

Race condition in eshell-gather-process-output

[Jürgen Hötzel]

Hi,

I stumbled across this while debugging eshell/tramp external commands:

In eshell-gather-process-output the external process is started using
start-file-process:

      (setq proc
	    (let ((process-connection-type
		   (unless (eshell-needs-pipe-p command)
		     process-connection-type))
		  (command (or (file-remote-p command 'localname) command)))
	      (apply 'start-file-process
		     (file-name-nondirectory command) nil
		     ;; `start-process' can't deal with relative filenames.
		     (append (list (expand-file-name command)) args))))
----------------------------------------------------------------------------------------
something can happen meanwhile: The process may have written
output (nil output buffer)
----------------------------------------------------------------------------------------

      (eshell-record-process-object proc)
      (set-process-buffer proc (current-buffer))

^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Maybe too late to set output buffer!

Why is the output buffer not set directly:

(apply 'start-file-process
		     (file-name-nondirectory command) (current-buffer)
		     ...)

Regards,

Jürgen

Re: Race condition in eshell-gather-process-output

[Andreas Schwab]

Jürgen Hötzel <juergen@archlinux.org> writes:

> 	      (apply 'start-file-process
> 		     (file-name-nondirectory command) nil
> 		     ;; `start-process' can't deal with relative filenames.
> 		     (append (list (expand-file-name command)) args))))
> ----------------------------------------------------------------------------------------
> something can happen meanwhile: The process may have written
> output (nil output buffer)
> ----------------------------------------------------------------------------------------
>
>       (eshell-record-process-object proc)
>       (set-process-buffer proc (current-buffer))
>
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Maybe too late to set output buffer!

It isn't.  Process output isn't read until accept-process-output is
called.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Race condition in eshell-gather-process-output

[Jürgen Hötzel]

On Sat, Jan 19, 2013 at 1:16 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Jürgen Hötzel <juergen@archlinux.org> writes:
>
>>             (apply 'start-file-process
>>                    (file-name-nondirectory command) nil
>>                    ;; `start-process' can't deal with relative filenames.
>>                    (append (list (expand-file-name command)) args))))
>> ----------------------------------------------------------------------------------------
>> something can happen meanwhile: The process may have written
>> output (nil output buffer)
>> ----------------------------------------------------------------------------------------
>>
>>       (eshell-record-process-object proc)
>>       (set-process-buffer proc (current-buffer))
>>
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> Maybe too late to set output buffer!
>
> It isn't.  Process output isn't read until accept-process-output is
> called.

I did this test in Eshell:

~ $ bash -c "echo test1 && sleep 1 && echo test2"
test1
test2
~ $

Than inserting a delay in obove code:


....
	      (apply 'start-file-process
		     (file-name-nondirectory command) nil
		     ;; `start-process' can't deal with relative filenames.
		     (append (list (expand-file-name command)) args))))
      (sleep-for 1)
      (eshell-record-process-object proc)
....

results in:

~ $ bash -c "echo test1 && sleep 1 && echo test2"
test2
~ $

missing the first line of output

Regards,

Jürgen

Re: Race condition in eshell-gather-process-output

[Andreas Schwab]

Jürgen Hötzel <juergen@hoetzel.info> writes:

>       (sleep-for 1)

sleep-for calls wait_reading_process_output (which is the implementation
of accept-process-output).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."