bug#35506: 27.0.50; Emacs hangs while interrupting tramp processes

[John Shahid <jvshahid@gmail.com>]

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

> John Shahid <jvshahid@gmail.com> writes:
>
> Hi John,
>
>>> So your patch is correct, pls push.
>>
>> Thanks for reviewing the patch.  Unfortunately, I don't have push access
>> to the repository.  Do you mind pushing the patch for me?
>
> Done, pushed to master.

Thank you.

>> 1. Asynchronously kill the process (the first patch)
>> 2. Always return success, e.g. return proc.
>>
>> It doesn't make sense to use the default interrupt function for remote
>> processes anyway, so always succeeding seems like the right thing to do
>> here.  This begs the question, why do we have to wait for the process
>> output at all?
>
> Well, the caller wants to know whether `interrupt-process' succeeded.

I think we are interpreting success differently in this case.  My
interpretation of success that the following two conditions are met:

1. The process is a remote process
2. The kill command runs successfully.  In other words the signal is
delivered

It is up to the process to decide whether it should exit or not.  For
example a process could interpret SIGINT to mean dump some debugging
information to the terminal.  That should not affect the return value of
`tramp-interrupt-process' since it achieved the goal of delivering the
signal.  Another data point is `internal-default-interrupt-process'.
This function calls `kill' and return a success value, regardless of
whether the process exits or not.

The previous paragraph outlines my thought process which lead me to the
change I proposed in the initial patch.

>>> So we must investigate, why `interrupt-process' does not return in
>>> your case.
>>
>> That is a good point.  I didn't look deeply into why the `with-timeout'
>> isn't timing out in my case.  I will try to understand what is going on
>> in the next few days.
>
> IIRC, `tramp-accept-process-output' suppresses timers. So we might
> change the code to (untested)

I think that explains the issue I was running into.

> 	;; Wait, until the process has disappeared.  If it doesn't,
> 	;; fall back to the default implementation.
> 	(and (tramp-accept-process-output proc 1)
> 	     ;; Report success.
> 	     proc)))))
>
> Does this work for you?

I will test it out tomorrow morning.  I still prefer not waiting at all.
I am not sure if the 1 second wait will be noticeable or not, but we
wouldn't know until I try it out.

Cheers,

JS