bug#49954: 28.0.50; TRAMP: cannot kill child processes: "Forbidden reentrant call of Tramp"

[Michael Albinus <michael.albinus@gmx.de>]

Dima Kogan <dima@secretsauce.net> writes:

> Hi Michael.

Hi Dima,

> I did some debugging, and it appears that the tramp property-caching
> mechanism is failing. We exit the (with-tramp-locked-connection ...)
> form, but when we try to enter the next (with-tramp-locked-connection
> ...) form, it looks locked because
>
> (tramp-get-connection-property proc "locked" nil)
>
> is evaluating to t. I instrumented (tramp-get-connection-property), and
> I can see that this t comes from the property cache. I can "fix" the bug
> by removing the
>
>     (when (and (not (eq cached tramp-cache-undefined))
> 	       ;; If the key is an auxiliary process object, check
> 	       ;; whether the process is still alive.
> 	       (not (and (processp key) (not (process-live-p key)))))
>       (setq value cached
> 	    cache-used t))
>
> form from (tramp-get-connection-property)
>
> Can I get the intent of this form? Are you trying to use this form if
> the process is alive, or if the process is dead? My process is very much
> alive, so this form is being used. Is this what we want?

No, I believe the mechanism is working correctly. A lock is placed on
the connection process of Tramp, and it is kept until the related
operation has finished. The process is expected to be alive, the
additional check of process-live-p is just a reassurance.

> If it is what we want, then the cached value of t is the problem. I
> haven't looked into why that's happening yet.

The problem is the following: Tramp sends a (shell) command to the
remote host, and waits for the reply. This must be atomic, no other
command shall be sent in parallel, in order to get the proper reply.

If there is asynchronous code running, from a timer, a process filter or
sentinel, or process interrupt, this can happen: a second command is
sent, while the other command didn't get its reply yet. The macro
with-tramp-locked-connection shall protect us in this case, and it
raises the respective error. That's not a perfect solution. A better way
would do use threads with mutexes, so that the second command can wait
until the first command got its reply. Something like this needs to be
implemented.

> Thanks!

Best regards, Michael.