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

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

[John Shahid]

There are two issues I ran into recently while trying to interrupt a
remote process spawned by tramp.  The process in question is terraform
which ahss the following process tree:

  process1 (spawned by tramp)
  ⌞ process2 (child process)
    ⌞ process3 (multiple sub processes)
    ⌞ process4

process1 does not handle signals which causes the interrupts to be
ignored.  Moreover because of the following line in
`tramp-interrupt-process', Emacs ends up busy waiting until the process
exits:

  (with-timeout (1 (ignore))
    (while (tramp-accept-process-output proc))
    ;; Report success.
    proc)

It looks like the first issue can be solved by killing the entire
process group, but I am not sure how safe it is to always do that.  I
will attach a patch that I use locally which seem harmless so far.

The second issue can be solved by not waiting for the process to exit.
This seems to be in line with the contract of `interrupt-process'.  I
will attach a patch to remove the wait.


Note, the following commit is not on master.  This is a local commit
with a bunch of changes that I'm trying to get pushed to master.

In GNU Emacs 27.0.50 (build 22, x86_64-pc-linux-gnu, GTK+ Version 3.24.8)
 of 2019-04-29 built on amun
Repository revision: cf2d0e2e58a683756878c9c7e4dd2066efe6e807
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Arch Linux

Recent messages:
Mark saved where search started
[a-z..]:Set [SPC]:clear
Quit [2 times]
Winner undo (1 / 199)
[mu4e] Switched context to personal
(New file)
Mark set
Starting new Ispell process /usr/bin/ispell with default dictionary...
Invalid face attribute :inherit mu4e-header-key-face [17 times]
Message modified; kill anyway? (y or n) y

Configured using:
 'configure --prefix=/home/jvshahid/bin/emacs-27
 PKG_CONFIG_PATH=/home/jvshahid/.gvm/pkgsets/go1.11.1/global/overlay/lib/pkgconfig:'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS GLIB
NOTIFY INOTIFY ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM THREADS LIBSYSTEMD JSON PDUMPER
LCMS2 GMP

Important settings:
  value of $LC_COLLATE: en_US.UTF-8
  value of $LC_CTYPE: en_US.UTF-8
  value of $LC_MESSAGES: en_US.UTF-8
  value of $LC_MONETARY: en_US.UTF-8
  value of $LC_NUMERIC: en_US.UTF-8
  value of $LC_TIME: en_US.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: mu4e-headers

Minor modes in effect:
  global-magit-file-mode: t
  global-git-commit-mode: t
  hl-line-mode: t
  recentf-mode: t
  show-paren-mode: t
  display-battery-mode: t
  global-company-mode: t
  company-mode: t
  flx-ido-mode: t
  helm-mode: t
  async-bytecomp-package-mode: t
  winner-mode: t
  global-auto-revert-mode: t
  savehist-mode: t
  display-time-mode: t
  direnv-mode: t
  shell-dirtrack-mode: t
  projectile-mode: t
  straight-use-package-mode: t
  straight-package-neutering-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/jvshahid/.emacs.d/straight/build/soap-client/soap-client hides /home/jvshahid/bin/emacs-27/share/emacs/27.0.50/lisp/net/soap-client
/home/jvshahid/.emacs.d/straight/build/soap-client/soap-inspect hides /home/jvshahid/bin/emacs-27/share/emacs/27.0.50/lisp/net/soap-inspect
/home/jvshahid/.emacs.d/straight/build/cl-lib/cl-lib hides /home/jvshahid/bin/emacs-27/share/emacs/27.0.50/lisp/emacs-lisp/cl-lib
/home/jvshahid/.emacs.d/straight/build/let-alist/let-alist hides /home/jvshahid/bin/emacs-27/share/emacs/27.0.50/lisp/emacs-lisp/let-alist
/home/jvshahid/.emacs.d/straight/build/seq/seq hides /home/jvshahid/bin/emacs-27/share/emacs/27.0.50/lisp/emacs-lisp/seq
/home/jvshahid/.emacs.d/straight/build/cl-generic/cl-generic hides /home/jvshahid/bin/emacs-27/share/emacs/27.0.50/lisp/emacs-lisp/cl-generic

Features:
(shadow sort mail-extr emacsbug flyspell ispell supercite regi
face-remap eieio-opt wgrep tramp-archive tramp-gvfs zeroconf dbus novice
cl-print debug help-fns company-robe flycheck robe url-http url-auth
url-gw inf-ruby hideshow cap-words superword subword ruby-mode
ace-window avy pcmpl-unix bug-reference timezone tabify org-capture
org-archive helm-org shr-color shr svg helm-misc helm-ring org-duration
diary-lib diary-loaddefs cal-iso org-indent org-rmail org-mhe org-irc
org-info org-gnus nnir org-docview org-bibtex bibtex org-bbdb org-w3m
org-agenda conf-mode tramp-cmds magit-extras magit-bookmark
magit-submodule magit-obsolete magit-blame magit-stash magit-bisect
magit-push magit-pull magit-fetch magit-clone magit-remote magit-commit
magit-sequence magit-notes magit-worktree magit-tag magit-merge
magit-branch magit-reset magit-files magit-refs magit-status magit
magit-repos magit-apply magit-wip magit-log which-func magit-diff
smerge-mode magit-core magit-autorevert magit-margin magit-transient
magit-process magit-mode transient git-commit magit-git magit-section
magit-utils crm log-edit pcvs-util add-log with-editor dired-aux vc-git
diff-mode helm-eshell skeleton dabbrev org-mu4e org-element avl-tree org
org-macro org-footnote org-pcomplete org-list org-faces org-entities
org-version ob-emacs-lisp ob ob-tangle org-src ob-ref ob-lob ob-table
ob-keys ob-exp ob-comint ob-core ob-eval org-compat org-macs
org-loaddefs mu4e desktop frameset mu4e-speedbar speedbar sb-image
ezimage dframe mu4e-main mu4e-view cal-menu calendar cal-loaddefs
gnus-art mm-uu mml2015 mm-view mml-smime smime dig gnus-sum gnus-group
gnus-undo gnus-start gnus-cloud nnimap nnmail mail-source utf7 netrc
nnoo gnus-spec gnus-int gnus-range gnus-win gnus nnheader mu4e-headers
mu4e-compose mu4e-context mu4e-draft mu4e-actions rfc2368 smtpmail
sendmail mu4e-mark mu4e-message flow-fill mu4e-proc mu4e-utils doc-view
image-mode mu4e-lists mu4e-vars message rfc822 mml mml-sec epa derived
epg gnus-util rmail rmail-loaddefs text-property-search mm-decode
mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util
ietf-drums mail-prsvr mailabbrev gmm-utils mailheader hl-line mu4e-meta
two-column iso-transl rng-xsd xsd-regexp rng-cmpct rng-nxml rng-valid
rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn
nxml-ns nxml-mode nxml-outln nxml-rap sgml-mode dom nxml-util nxml-enc
xmltok sh-script smie executable recentf helm-for-files helm-bookmark
helm-adaptive bookmark pp helm-external helm-net browse-url xml misearch
multi-isearch helm-command helm-elisp helm-eval edebug backtrace
helm-info time-date jka-compr eshell-z windmove ffap em-unix em-term
term disp-table ehelp em-script em-prompt em-ls em-hist em-pred em-dirs
esh-var em-cmpl em-basic em-banner em-alias esh-mode eshell esh-cmd
esh-ext esh-opt esh-proc esh-io esh-arg esh-module esh-groups
parinfer-ext paredit parinfer ediff-merg ediff-wind ediff-diff
ediff-mult ediff-help ediff-init ediff-util ediff mode-local find-func
parinferlib dired-x dired dired-loaddefs paren battery cus-start
cus-load floobits-autoloads highlight-autoloads finder-inf
company-terraform-autoloads terraform-mode-autoloads hcl-mode-autoloads
arduino-mode-autoloads ede/auto lsp-java request mail-utils url
url-proxy url-privacy url-expand url-methods url-history url-cookie
url-domsuf mailcap lsp lsp-mode markdown-mode edit-indirect color
noutline outline tree-widget wid-edit url-util spinner network-stream
puny nsm rmc inline imenu ht f s em-glob esh-util dash-functional
flymake-proc flymake warnings cc-mode cc-fonts cc-guess cc-menus cc-cmds
cc-styles cc-align cc-engine cc-vars cc-defs lsp-java-autoloads
request-autoloads rust-mode-autoloads haskell-mode-autoloads
flycheck-clojure-autoloads cider-autoloads sesman-autoloads
queue-autoloads clojure-mode-autoloads parinfer-autoloads
paredit-autoloads ginkgo-mode-autoloads go-rename-autoloads
company-go-autoloads go-eldoc-autoloads go-guru-autoloads
go-mode-autoloads robe-autoloads inf-ruby-autoloads rvm-autoloads
company-lsp-autoloads lsp-ui-autoloads lsp-mode-autoloads
spinner-autoloads ht-autoloads f-autoloads dash-functional-autoloads
company-oddmuse company-keywords company-etags etags fileloop generator
xref project company-gtags company-dabbrev-code company-dabbrev
company-files company-capf company-cmake company-xcode company-clang
company-semantic company-eclim company-template company-bbdb company
company-autoloads emms-autoloads pianobar-autoloads eshell-z-autoloads
cl flx-ido flx flx-ido-autoloads flx-autoloads helm-mode helm-config
helm-easymenu async-bytecomp helm-projectile helm-files helm-tags
helm-buffers helm-occur helm-grep helm-regexp helm-utils helm-locate
helm-help helm-types helm helm-source eieio-compat helm-multi-match
helm-lib async helm-projectile-autoloads helm-autoloads
helm-core-autoloads popup-autoloads git-link-autoloads magit-autoloads
transient-autoloads git-commit-autoloads winner golden-ratio
golden-ratio-autoloads emacs-rotate-autoloads exwm-randr xcb-randr
exwm-config ido exwm exwm-input xcb-keysyms xcb-xkb exwm-manage
exwm-floating xcb-cursor xcb-render exwm-layout exwm-workspace exwm-core
xcb-ewmh xcb-icccm xcb xcb-xproto xcb-types xcb-debug exwm-autoloads
xelb-autoloads cl-generic-autoloads tango-dark-theme server edmacro
kmacro autorevert filenotify savehist time direnv dash tramp-cache
tramp-sh tramp tramp-loaddefs trampver tramp-integration files-x
tramp-compat ucs-normalize shell pcomplete parse-time format-spec advice
projectile grep ibuf-ext ibuffer ibuffer-loaddefs direnv-autoloads
with-editor-autoloads async-autoloads edit-indirect-autoloads
concourse-mode-autoloads hierarchy-autoloads wgrep-autoloads
flycheck-autoloads seq-autoloads let-alist-autoloads dash-autoloads
ace-window-autoloads avy-autoloads dockerfile-mode-autoloads s-autoloads
yasnippet-snippets-autoloads yasnippet-autoloads protobuf-mode-autoloads
markdown-mode-autoloads yaml-mode-autoloads etags-select-autoloads
projectile-autoloads pkg-info-autoloads epl-autoloads debbugs-autoloads
soap-client-autoloads cl-lib-autoloads straight-autoloads rx compile
comint ansi-color ring cl-extra straight info autoload radix-tree
easy-mmode pcase checkdoc lisp-mnt thingatpt help-mode elec-pair
mule-util package easymenu epg-config url-handlers url-parse auth-source
cl-seq eieio eieio-core cl-macs eieio-loaddefs password-cache json
subr-x map url-vars seq byte-opt gv bytecomp byte-compile cconv
cl-loaddefs cl-lib tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 1216656 1103680)
 (symbols 48 59970 35)
 (strings 32 281872 133497)
 (string-bytes 1 10869916)
 (vectors 16 140937)
 (vector-slots 8 2037180 840586)
 (floats 8 1188 2973)
 (intervals 56 67788 22723)
 (buffers 992 210))

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

[John Shahid]

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


Attached are the two patches I mentioned in the bug report.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Kill-the-entire-process-group.patch --]
[-- Type: text/x-patch, Size: 1019 bytes --]

From e056e98d4b899b8ae7636a967d0a55125e624fad Mon Sep 17 00:00:00 2001
From: John Shahid <jvshahid@gmail.com>
Date: Mon, 29 Apr 2019 16:09:32 -0400
Subject: [PATCH] Kill the entire process group

Some processes (e.g. terraform) don't handle signals but its
subprocess (with the exact same name) does.  Killing the parent
process doesn't do anything, we have to send the signal to the entire
group in order to reach the child.  This seems to be what terminals
typically do.
---
 lisp/net/tramp.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 2e1a0960d7..427b3c41e3 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -4870,7 +4870,7 @@ tramp-interrupt-process
 	(tramp-compat-funcall
 	 'tramp-send-command
 	 (process-get proc 'vector)
-	 (format "kill -2 %d" pid))
+	 (format "kill -2 -%d" pid))
 	;; Wait, until the process has disappeared.  If it doesn't,
 	;; fall back to the default implementation.
 	(with-timeout (1 (ignore))
-- 
2.21.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Do-not-wait-for-process-output-after-interruption.patch --]
[-- Type: text/x-patch, Size: 1170 bytes --]

From cf2d0e2e58a683756878c9c7e4dd2066efe6e807 Mon Sep 17 00:00:00 2001
From: John Shahid <jvshahid@gmail.com>
Date: Mon, 29 Apr 2019 16:10:47 -0400
Subject: [PATCH] Do not wait for process output after interruption.

Some processes such as terraform can have cleanup logic that causes
the process to take longer to shut down than normal.  This causes
eshell to do a busy wait locking up Emacs while waiting for the
process to finish.  I don't understand why we do that anyway, just let
the process finish at its own pace.
---
 lisp/net/tramp.el | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 427b3c41e3..27bef45efb 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -4873,10 +4873,7 @@ tramp-interrupt-process
 	 (format "kill -2 -%d" pid))
 	;; Wait, until the process has disappeared.  If it doesn't,
 	;; fall back to the default implementation.
-	(with-timeout (1 (ignore))
-	  (while (tramp-accept-process-output proc))
-	  ;; Report success.
-	  proc)))))
+        ))))
 
 ;; `interrupt-process-functions' exists since Emacs 26.1.
 (when (boundp 'interrupt-process-functions)
-- 
2.21.0

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

[Michael Albinus]

John Shahid <jvshahid@gmail.com> writes:

Hi John,

> It looks like the first issue can be solved by killing the entire
> process group, but I am not sure how safe it is to always do that.  I
> will attach a patch that I use locally which seem harmless so far.

Reading the Emacs manual again at (info "(elisp) Signals to Processes")

--8<---------------cut here---------------start------------->8---
If CURRENT-GROUP is ‘nil’, the signal is sent to the process group of
the immediate subprocess of Emacs.
--8<---------------cut here---------------end--------------->8---

So your patch is correct, pls push.

> The second issue can be solved by not waiting for the process to exit.
> This seems to be in line with the contract of `interrupt-process'.  I
> will attach a patch to remove the wait.

No, Tramp must report success. See the Emacs manual, same location as
above:

--8<---------------cut here---------------start------------->8---
 -- Variable: interrupt-process-functions
     This variable is a list of functions to be called for
     ‘interrupt-process’.  The arguments of the functions are the same
     as for ‘interrupt-process’.  These functions are called in the
     order of the list, until one of them returns non-‘nil’.  The
     default function, which shall always be the last in this list, is
     ‘internal-default-interrupt-process’.

     This is the mechanism, how Tramp implements ‘interrupt-process’.
--8<---------------cut here---------------end--------------->8---

So we must investigate, why `interrupt-process' does not return in your case.

Best regards, Michael.

PS: In the FSF list, your email address is given as <vshahid@gmail.com>.
Looks like a typo, maybe you contact the FSF clerk about.

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

[John Shahid]

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

> John Shahid <jvshahid@gmail.com> writes:
>
> Hi John,

[...]

> Reading the Emacs manual again at (info "(elisp) Signals to Processes")
>
> --8<---------------cut here---------------start------------->8---
> If CURRENT-GROUP is ‘nil’, the signal is sent to the process group of
> the immediate subprocess of Emacs.
> --8<---------------cut here---------------end--------------->8---
>
> 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?

[...]

> No, Tramp must report success. See the Emacs manual, same location as
> above:
>
> --8<---------------cut here---------------start------------->8---
>  -- Variable: interrupt-process-functions
>      This variable is a list of functions to be called for
>      ‘interrupt-process’.  The arguments of the functions are the same
>      as for ‘interrupt-process’.  These functions are called in the
>      order of the list, until one of them returns non-‘nil’.  The
>      default function, which shall always be the last in this list, is
>      ‘internal-default-interrupt-process’.
>
>      This is the mechanism, how Tramp implements ‘interrupt-process’.
> --8<---------------cut here---------------end--------------->8---

I just realized that I made an unintentional change.  I got rid of the
return value, so the function will always return nil causing
`interrupt-process' to fallback to the default implementation.  That was
not my intention.  I think what I meant to do is to:

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?

> 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.

> Best regards, Michael.
>
> PS: In the FSF list, your email address is given as <vshahid@gmail.com>.
> Looks like a typo, maybe you contact the FSF clerk about.

Thanks for letting me know.  No one brought this up before.  I will
follow up with the FSF clerk.

Cheers,

JS

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

[Michael Albinus]

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.

> 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.

>> 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)

	;; 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?

> Cheers,
>
> JS

Best regards, Michael.

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

[John Shahid]

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

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

[John Shahid]

John Shahid <jvshahid@gmail.com> writes:


[...]

>> 	;; 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.

I just tried that patch and it fixed the issue.  The 1 second timeout
isn't noticeable at all.  I'm happy to create a patch and attach it to
the bug report.

Cheers,

JS

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

[Michael Albinus]

John Shahid <jvshahid@gmail.com> writes:

Hi John,

>> 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.
>
> I just tried that patch and it fixed the issue.  The 1 second timeout
> isn't noticeable at all.  I'm happy to create a patch and attach it to
> the bug report.

Could it be that we're waiting on each other? Do you plan to contribute
a patch, or shall I do?

(There's no rush with this, and I don't want to urge you. Just for
clarification.)

> Cheers,
>
> JS

Best regards, Michael.

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

[John Shahid]

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


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

> John Shahid <jvshahid@gmail.com> writes:
>
> Hi John,
>
>>> 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.
>>
>> I just tried that patch and it fixed the issue.  The 1 second timeout
>> isn't noticeable at all.  I'm happy to create a patch and attach it to
>> the bug report.
>
> Could it be that we're waiting on each other? Do you plan to contribute
> a patch, or shall I do?

Indeed.  I went ahead and created the patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Avoid-infinitly-looping-in-tramp-interrupt-process.patch --]
[-- Type: text/x-patch, Size: 1167 bytes --]

From 418048bb52188a747acab6349a71c42c4ceebe2b Mon Sep 17 00:00:00 2001
From: John Shahid <jvshahid@gmail.com>
Date: Mon, 29 Apr 2019 16:10:47 -0400
Subject: [PATCH] Avoid infinitly looping in tramp-interrupt-process

* lisp/net/tramp.el (tramp-interrupt-process): Remove with-timeout.
Instead pass a timeout to tramp-accept-process-output.
tramp-accept-process-output stops timers from running which makes the
with-timeout ineffective.
---
 lisp/net/tramp.el | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 0a3129fd45..2aa62eba80 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -4861,10 +4861,9 @@ tramp-interrupt-process
 	 (format "kill -2 -%d" pid))
 	;; Wait, until the process has disappeared.  If it doesn't,
 	;; fall back to the default implementation.
-	(with-timeout (1 (ignore))
-	  (while (tramp-accept-process-output proc))
-	  ;; Report success.
-	  proc)))))
+        (and (tramp-accept-process-output proc 1)
+	     ;; Report success.
+	     proc)))))
 
 ;; `interrupt-process-functions' exists since Emacs 26.1.
 (when (boundp 'interrupt-process-functions)
-- 
2.21.0

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

[Michael Albinus]

John Shahid <jvshahid@gmail.com> writes:

Hi John,

>>> I just tried that patch and it fixed the issue.  The 1 second timeout
>>> isn't noticeable at all.  I'm happy to create a patch and attach it to
>>> the bug report.
>>
>> Could it be that we're waiting on each other? Do you plan to contribute
>> a patch, or shall I do?
>
> Indeed.  I went ahead and created the patch.

Thanks. I've pushed it to master, closing the bug.

Best regards, Michael.