bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[sbaugh]

1. emacs -Q under X, preferably X forwarded over ssh to make things slower.
2. (setq save-interprogram-paste-before-kill 2000) (or any other integer)
3. Copy some very large data in another X client, so the selection is
very large.
4. (kill-new "foo")
5. Observe Emacs hanging as it receives the entire large data from the
selection owner, and then after receiving it all, discards it because
it's more than 2000 bytes.

Solution: receive_incremental_selection in xselect.c should support a
cap on the size of the selection it receives and truncate (or discard,
returning nil?) the selection if it's larger than that.  And setting
save-interprogram-paste-before-kill to a numeric value should activate
this cap.



In GNU Emacs 29.0.92 (build 2, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw3d scroll bars) of 2023-07-01 built on earth
Repository revision: b179926388ee76f7b3304535a7189f89bd7c7f8c
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12014000
System Description: NixOS 22.11 (Raccoon)

Configured using:
 'configure --with-x-toolkit=lucid --with-tree-sitter'


Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY
PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS
TREE_SITTER WEBP X11 XAW3D XDBE XIM XPM LUCID ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Help

Minor modes in effect:
  TeX-PDF-mode: t
  pixel-scroll-precision-mode: t
  envrc-global-mode: t
  envrc-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  shell-dirtrack-mode: t
  server-mode: t
  windmove-mode: t
  savehist-mode: t
  save-place-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tab-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  isearch-fold-quotes-mode: t
  context-menu-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  buffer-read-only: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/sbaugh/.emacs.d/elpa/transient-0.3.7/transient hides /home/sbaugh/src/emacs/emacs-29/lisp/transient

Features:
(completion nndoc gnus-dup mm-archive debbugs-gnu debbugs-compat debbugs
soap-client rng-xsd rng-dt rng-util xsd-regexp debbugs-browse octave
semantic/symref/grep semantic/symref semantic/util-modes semantic/util
semantic semantic/tag semantic/lex semantic/fw mode-local cedet
org-archive org-attach org-agenda org-capture display-line-numbers
ibuffer ibuffer-loaddefs tabify em-tramp em-rebind em-smart em-unix
em-term term ehelp em-script em-prompt em-ls em-hist em-pred em-glob
em-extpipe em-cmpl em-dirs esh-var 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 esh-util goto-addr man cus-theme eieio-custom xwidget
magit-bookmark bookmark wid-browse tree-widget icon conf-mode descr-text
cus-edit pcmpl-unix pcmpl-gnu shadow emacsbug misc dabbrev cl-print
emacs-news-mode tex-info tex texmathp texinfo texinfo-loaddefs
bug-reference org-element org-persist org-id org-refile avl-tree
oc-basic ol-eww eww xdg url-queue mm-url ol-rmail ol-mhe ol-irc ol-info
ol-gnus nnselect ol-docview doc-view image-mode exif ol-bibtex bibtex
ol-bbdb ol-w3m ol-doi org-link-doi org org-macro org-pcomplete org-list
org-footnote org-faces org-entities noutline outline ob-python python ob
ob-tangle org-src ob-ref ob-lob ob-table ob-exp ob-comint ob-emacs-lisp
ob-core ob-eval org-cycle org-table ol org-fold org-fold-core org-keys
oc org-loaddefs cal-menu calendar cal-loaddefs org-version org-compat
org-macs etags fileloop generator find-dired pulse color js cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs grep vc-hg vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs log-view
vc nix-mode nix-repl nix-shell nix-store nix-log nix-instantiate
nix-shebang nix-format nix rust-ts-mode c-ts-common sh-script smie
treesit executable dired-aux dired-x tramp-archive tramp-gvfs tramp
tramp-loaddefs trampver tramp-integration files-x tramp-compat cus-start
cus-load pixel-scroll cua-base misearch multi-isearch vc-git
vc-dispatcher sort smiley gnus-cite mail-extr textsec uni-scripts
idna-mapping ucs-normalize uni-confusable textsec-check qp gnus-async
gnus-bcklg gnus-ml disp-table nndraft nnmh nnfolder gnus-agent gnus-srvr
gnus-score score-mode nnvirtual gnus-msg gnus-art mm-uu mml2015 mm-view
mml-smime smime dig nntp gnus-cache gnus-sum shr pixel-fill kinsoku
url-file svg dom gnus-group gnus-undo gnus-start gnus-dbus dbus xml
gnus-cloud nnimap nnmail mail-source utf7 nnoo gnus-spec gnus-int
gnus-range gnus-win gnus nnheader range wid-edit timezone parse-time
iso8601 mule-util jka-compr eglot external-completion array jsonrpc ert
pp ewoc debug backtrace find-func xref flymake-proc flymake warnings
icons compile project network-stream url-http url-gw nsm url-cache
url-auth face-remap ffap shortdoc desktop frameset help-fns radix-tree
lui-autopaste circe advice lui-irc-colors irc gnutls lcs lui-logging
lui-format lui tracking shorten thingatpt flyspell ispell circe-compat
agda2 envrc inheritenv page-ext magit-extras magit-submodule
magit-obsolete magit-blame magit-stash magit-reflog 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 imenu magit-diff
smerge-mode diff diff-mode easy-mmode git-commit rx log-edit message
sendmail yank-media puny dired dired-loaddefs rfc822 mml mml-sec epa
derived epg rfc6068 epg-config gnus-util text-property-search time-date
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util
ietf-drums mail-prsvr mailabbrev mail-utils gmm-utils mailheader
pcvs-util add-log magit-core magit-autorevert autorevert filenotify
magit-margin magit-transient magit-process with-editor shell pcomplete
comint ansi-osc ring server ansi-color magit-mode transient cl-extra
edmacro kmacro help-mode format-spec magit-git magit-section magit-utils
crm dash windmove modus-vivendi-theme modus-themes pcase savehist
saveplace finder-inf auctex-autoloads tex-site circe-autoloads
csv-mode-autoloads cyberpunk-theme-autoloads debbugs-autoloads
eat-autoloads envrc-autoloads ggtags-autoloads
graphviz-dot-mode-autoloads htmlize-autoloads inheritenv-autoloads
magit-autoloads git-commit-autoloads mentor-autoloads async-autoloads
nix-mode-autoloads magit-section-autoloads dash-autoloads
notmuch-autoloads rust-mode-autoloads transient-autoloads
url-scgi-autoloads vundo-autoloads info with-editor-autoloads
xml-rpc-autoloads package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie generate-lisp-file
url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs password-cache json subr-x map byte-opt gv
bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc iso-transl tooltip
cconv eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode 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 lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow
isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine 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 emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting system-font-setting font-render-setting cairo x-toolkit
x multi-tty make-network-process emacs)

Memory information:
((conses 16 1334240 144878)
 (symbols 48 62872 2)
 (strings 32 278128 12807)
 (string-bytes 1 11041307)
 (vectors 16 132517)
 (vector-slots 8 2850991 257159)
 (floats 8 749 666)
 (intervals 56 83547 2259)
 (buffers 984 140))

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

sbaugh@catern.com writes:

> 1. emacs -Q under X, preferably X forwarded over ssh to make things slower.
> 2. (setq save-interprogram-paste-before-kill 2000) (or any other integer)
> 3. Copy some very large data in another X client, so the selection is
> very large.
> 4. (kill-new "foo")
> 5. Observe Emacs hanging as it receives the entire large data from the
> selection owner, and then after receiving it all, discards it because
> it's more than 2000 bytes.
>
> Solution: receive_incremental_selection in xselect.c should support a
> cap on the size of the selection it receives and truncate (or discard,
> returning nil?) the selection if it's larger than that.  And setting
> save-interprogram-paste-before-kill to a numeric value should activate
> this cap.

This is not a bug because you can quit while Emacs is waiting for
selection data to arrive from the owner.  The ICCCM provides no means
for the requestor to terminate an incremental selection transfer before
it completes, and owners may become confused if Emacs abruptly stops
responding to their property changes while a transfer is in progress.
Emacs should avoid doing so unless the user explicitly quits (pointing
to a problem with the selection owner anyway.)

Additionally, the way we deal with potentially long-running data
transfer operations in Emacs is not to apply a limit on the size of the
data being transferred, but to make quitting possible within those
transfers.  What may be slow for you can in fact be perfectly acceptable
for others who are connected to their X servers through faster
connections.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Spencer Baugh]

Po Lu <luangruo@yahoo.com> writes:
> sbaugh@catern.com writes:
>
>> 1. emacs -Q under X, preferably X forwarded over ssh to make things slower.
>> 2. (setq save-interprogram-paste-before-kill 2000) (or any other integer)
>> 3. Copy some very large data in another X client, so the selection is
>> very large.
>> 4. (kill-new "foo")
>> 5. Observe Emacs hanging as it receives the entire large data from the
>> selection owner, and then after receiving it all, discards it because
>> it's more than 2000 bytes.
>>
>> Solution: receive_incremental_selection in xselect.c should support a
>> cap on the size of the selection it receives and truncate (or discard,
>> returning nil?) the selection if it's larger than that.  And setting
>> save-interprogram-paste-before-kill to a numeric value should activate
>> this cap.
>
> This is not a bug because you can quit while Emacs is waiting for
> selection data to arrive from the owner.

When you do that, you interrupt the operation which is trying to add a
new kill.  If you interrupt it and try again, you'll just get the same
long delay again.  There's no way to mitigate this from within Emacs,
other than by turning off save-interprogram-paste-before-kill.

> The ICCCM provides no means for the requestor to terminate an
> incremental selection transfer before it completes, and owners may
> become confused if Emacs abruptly stops responding to their property
> changes while a transfer is in progress.

Is there really no way to avoid a large data transfer in ICCCM?

Is there some way to learn the size of the selection in advance and then
decide not to read it if it's too large?  That would also fit the spec
of save-interprogram-paste-before-kill.

> Emacs should avoid doing so unless the user explicitly quits (pointing
> to a problem with the selection owner anyway.)
>
> Additionally, the way we deal with potentially long-running data
> transfer operations in Emacs is not to apply a limit on the size of
> the data being transferred, but to make quitting possible within those
> transfers.  What may be slow for you can in fact be perfectly
> acceptable for others who are connected to their X servers through
> faster connections.

Yes, that's why this is a customizable setting rather than hard-coded.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Spencer Baugh <sbaugh@janestreet.com> writes:

> When you do that, you interrupt the operation which is trying to add a
> new kill.  If you interrupt it and try again, you'll just get the same
> long delay again.  There's no way to mitigate this from within Emacs,
> other than by turning off save-interprogram-paste-before-kill.

Then I guess the solution is to temporarily disable
`save-interprogram-paste-before-kill' if a quit arrives while it is
reading selection data.

> Is there really no way to avoid a large data transfer in ICCCM?

There is none.

> Is there some way to learn the size of the selection in advance 

Also no.  The selection owner may elect to provide a lower bound on the
size of the selection data when initializing INCR transfer, but the
requestor must be prepared for the owner to send transfer more data than
that.

> and then decide not to read it if it's too large?  That would also fit
> the spec of save-interprogram-paste-before-kill.

Once you start a selection transfer, it must complete 

>> Additionally, the way we deal with potentially long-running data
>> transfer operations in Emacs is not to apply a limit on the size of
>> the data being transferred, but to make quitting possible within those
>> transfers.  What may be slow for you can in fact be perfectly
>> acceptable for others who are connected to their X servers through
>> faster connections.
>
> Yes, that's why this is a customizable setting rather than hard-coded.

That still contradicts my previous remark: Emacs should not place limits
on the _amount_ of data transferred in the first place (except perhaps
to avoid running out of VM) but allow the user to quit from long-running
operations instead.

Consider the case where selection data is being transferred from an
owner connected to the X server over a connection with very high
latency.  Waiting for a single quantum of selection data to become
available (usually 64k) may still take several seconds, during which no
data has been read.  Because of that, limiting the amount of selection
data transferred will have no effect on this delay.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[sbaugh]

Po Lu <luangruo@yahoo.com> writes:

> Spencer Baugh <sbaugh@janestreet.com> writes:
>
>> When you do that, you interrupt the operation which is trying to add a
>> new kill.  If you interrupt it and try again, you'll just get the same
>> long delay again.  There's no way to mitigate this from within Emacs,
>> other than by turning off save-interprogram-paste-before-kill.
>
> Then I guess the solution is to temporarily disable
> `save-interprogram-paste-before-kill' if a quit arrives while it is
> reading selection data.

That would be a decent solution.  Although I'm not sure how we'd
implement it.  We want to, somehow, know that after a selection-transfer
has been aborted, we should not try to transfer that selection again.
Is that something we can check?  Whether the selection has changed,
without transferring it?

>> Is there really no way to avoid a large data transfer in ICCCM?
>
> There is none.
>
>> Is there some way to learn the size of the selection in advance 
>
> Also no.  The selection owner may elect to provide a lower bound on the
> size of the selection data when initializing INCR transfer, but the
> requestor must be prepared for the owner to send transfer more data than
> that.
>
>> and then decide not to read it if it's too large?  That would also fit
>> the spec of save-interprogram-paste-before-kill.
>
> Once you start a selection transfer, it must complete 

That is unfortunate.  That seems like a terrible omission...  An
important network protocol principle is "tell the client up front how
much data you are going to send"...

Anyway, there's still a possible solution: we could return control to
the user if the transfer is too large, and continue with the INCR
transfer in the background, just to satisfy this ICCCM requirement,
discarding the data as we receive it.  This would be straightforward in
a program with a normal event loop, but might be difficult in Emacs...

>>> Additionally, the way we deal with potentially long-running data
>>> transfer operations in Emacs is not to apply a limit on the size of
>>> the data being transferred, but to make quitting possible within those
>>> transfers.  What may be slow for you can in fact be perfectly
>>> acceptable for others who are connected to their X servers through
>>> faster connections.
>>
>> Yes, that's why this is a customizable setting rather than hard-coded.
>
> That still contradicts my previous remark: Emacs should not place limits
> on the _amount_ of data transferred in the first place (except perhaps
> to avoid running out of VM) but allow the user to quit from long-running
> operations instead.
>
> Consider the case where selection data is being transferred from an
> owner connected to the X server over a connection with very high
> latency.  Waiting for a single quantum of selection data to become
> available (usually 64k) may still take several seconds, during which no
> data has been read.  Because of that, limiting the amount of selection
> data transferred will have no effect on this delay.

If the round-trip latency is 500ms, then waiting for the first quantum
of selection data will take at least 500ms, yes.  Subsequent quanta will
also take at least 500ms each.  If the selection is large, there may be
many.  If there are 20, then kill-new will take 10 seconds.  But if we
can limit the amount of selection data transferred, kill-new will only
take 500ms.

Wait... am I missing something?  You're saying it's okay for the user to
interactively choose to interrupt an INCR transfer, even though that
will leave things in a bad state?  Couldn't we just do the same thing in
code, then?  Can we wrap a user-customizable with-timeout around
gui-get-selection?

I actually agree now: limiting the amount of data transferred makes no
sense for user experience.  But limiting the *time spent* transferring
data makes total sense!  Users are able to do that today: We should
allow users to automate that!

So I think some new save-interprogram-paste-before-kill-timeout variable
would work perfectly.  All it would do is something users are already
capable of doing, but without aborting the entire kill-new operation.
That seems perfect!

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

sbaugh@catern.com writes:

> Po Lu <luangruo@yahoo.com> writes:
>
>> Spencer Baugh <sbaugh@janestreet.com> writes:
>>
>>> When you do that, you interrupt the operation which is trying to add a
>>> new kill.  If you interrupt it and try again, you'll just get the same
>>> long delay again.  There's no way to mitigate this from within Emacs,
>>> other than by turning off save-interprogram-paste-before-kill.
>>
>> Then I guess the solution is to temporarily disable
>> `save-interprogram-paste-before-kill' if a quit arrives while it is
>> reading selection data.
>
> That would be a decent solution.  Although I'm not sure how we'd
> implement it.  We want to, somehow, know that after a selection-transfer
> has been aborted, we should not try to transfer that selection again.
> Is that something we can check?  Whether the selection has changed,
> without transferring it?

Emacs will probably assert ownership of the selection after the kill
takes place, anyway, so there is no need.

> That is unfortunate.  That seems like a terrible omission...  An
> important network protocol principle is "tell the client up front how
> much data you are going to send"...

It's an intentional omission: INCR data transfer is designed to work
even if the owner itself does not know much data will be sent.  For
example, if the selection data is being transferred from a pipe or
socket.

> Anyway, there's still a possible solution: we could return control to
> the user if the transfer is too large, and continue with the INCR
> transfer in the background, just to satisfy this ICCCM requirement,
> discarding the data as we receive it.  This would be straightforward in
> a program with a normal event loop, but might be difficult in Emacs...

It's straightforward in Emacs, since that's already how it responds to
selection requests from other clients.  But it's a bad idea: what if the
user requests another conversion from the same selection owner while the
transfer is in progress?  This is technically possible, but will need
Emacs to specify a different property in each ConvertSelection request,
which will lead to lots of needless InternAtom requests and round
trips...

> If the round-trip latency is 500ms, then waiting for the first quantum
> of selection data will take at least 500ms, yes.  Subsequent quanta will
> also take at least 500ms each.  If the selection is large, there may be
> many.  If there are 20, then kill-new will take 10 seconds.  But if we
> can limit the amount of selection data transferred, kill-new will only
> take 500ms.
>
> Wait... am I missing something?  You're saying it's okay for the user to
> interactively choose to interrupt an INCR transfer, even though that
> will leave things in a bad state?

Yes, because when the user choses to do so, it is already clear that
there is a problem with the selection owner.  Transferring a lot of data
is not a capital offense, and Emacs shouldn't condemn the selection
owner just because it does.

> Couldn't we just do the same thing in code, then?  Can we wrap a
> user-customizable with-timeout around gui-get-selection?
>
> I actually agree now: limiting the amount of data transferred makes no
> sense for user experience.  But limiting the *time spent* transferring
> data makes total sense!  Users are able to do that today: We should
> allow users to automate that!
>
> So I think some new save-interprogram-paste-before-kill-timeout variable
> would work perfectly.  All it would do is something users are already
> capable of doing, but without aborting the entire kill-new operation.
> That seems perfect!

You mean, x-selection-timeout?

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[sbaugh]

Po Lu <luangruo@yahoo.com> writes:
> sbaugh@catern.com writes:
>
>> Po Lu <luangruo@yahoo.com> writes:
>>
>>> Spencer Baugh <sbaugh@janestreet.com> writes:
>>>
>>>> When you do that, you interrupt the operation which is trying to add a
>>>> new kill.  If you interrupt it and try again, you'll just get the same
>>>> long delay again.  There's no way to mitigate this from within Emacs,
>>>> other than by turning off save-interprogram-paste-before-kill.
>>>
>>> Then I guess the solution is to temporarily disable
>>> `save-interprogram-paste-before-kill' if a quit arrives while it is
>>> reading selection data.
>>
>> That would be a decent solution.  Although I'm not sure how we'd
>> implement it.  We want to, somehow, know that after a selection-transfer
>> has been aborted, we should not try to transfer that selection again.
>> Is that something we can check?  Whether the selection has changed,
>> without transferring it?
>
> Emacs will probably assert ownership of the selection after the kill
> takes place, anyway, so there is no need.

Good point!  So maybe if a quit arrives while we're reading the
selection in kill-new, we should immediately take ownership of the
selection.  With an condition-case, perhaps.

Or... just ignore the quit.  If we're reading the selection in kill-new
and there's a quit, just proceed to doing the kill.

>> That is unfortunate.  That seems like a terrible omission...  An
>> important network protocol principle is "tell the client up front how
>> much data you are going to send"...
>
> It's an intentional omission: INCR data transfer is designed to work
> even if the owner itself does not know much data will be sent.  For
> example, if the selection data is being transferred from a pipe or
> socket.

Ah, I see.  Then, like pipes and sockets, it should really at least
support interrupting the transfer...

>> Anyway, there's still a possible solution: we could return control to
>> the user if the transfer is too large, and continue with the INCR
>> transfer in the background, just to satisfy this ICCCM requirement,
>> discarding the data as we receive it.  This would be straightforward in
>> a program with a normal event loop, but might be difficult in Emacs...
>
> It's straightforward in Emacs, since that's already how it responds to
> selection requests from other clients.  But it's a bad idea: what if the
> user requests another conversion from the same selection owner while the
> transfer is in progress?  This is technically possible, but will need
> Emacs to specify a different property in each ConvertSelection request,
> which will lead to lots of needless InternAtom requests and round
> trips...

Such costs only need to be paid if there are indeed multiple conversions
happening at the same time.  In the common case there's just one, so
there's no extra cost of adding the ability to have multiple ongoing
conversions.

Actually, how does this work today?  If an Emacs user quits a conversion
and then immediately starts a new one, that seems to work fine.  We
don't do different properties in each request.  I realize the protocol
doesn't support it, but doesn't that suggest that it's fine in practice
to interrupt a conversion...?

(Quitting a conversion then running another would be one way to have
multiple conversions at the same time.  Another way would be (related to
my other thread about call-process) to allow running Lisp while an
incremental selection transfer is ongoing.  I know that seems useless
but I really am of the opinion that every blocking operation in Emacs
which can take more than a few milliseconds should support running Lisp
while it blocks...)

>> If the round-trip latency is 500ms, then waiting for the first quantum
>> of selection data will take at least 500ms, yes.  Subsequent quanta will
>> also take at least 500ms each.  If the selection is large, there may be
>> many.  If there are 20, then kill-new will take 10 seconds.  But if we
>> can limit the amount of selection data transferred, kill-new will only
>> take 500ms.
>>
>> Wait... am I missing something?  You're saying it's okay for the user to
>> interactively choose to interrupt an INCR transfer, even though that
>> will leave things in a bad state?
>
> Yes, because when the user choses to do so, it is already clear that
> there is a problem with the selection owner.  Transferring a lot of data
> is not a capital offense, and Emacs shouldn't condemn the selection
> owner just because it does.
>
>> Couldn't we just do the same thing in code, then?  Can we wrap a
>> user-customizable with-timeout around gui-get-selection?
>>
>> I actually agree now: limiting the amount of data transferred makes no
>> sense for user experience.  But limiting the *time spent* transferring
>> data makes total sense!  Users are able to do that today: We should
>> allow users to automate that!
>>
>> So I think some new save-interprogram-paste-before-kill-timeout variable
>> would work perfectly.  All it would do is something users are already
>> capable of doing, but without aborting the entire kill-new operation.
>> That seems perfect!
>
> You mean, x-selection-timeout?

Yes.  Personally I'd like to have a lower value for that when a
save-interprogram-paste-before-kill is triggered.  So adding a separate
save-interprogram-paste-before-kill-timeout which can be lower, and
which let-binds x-selection-timeout, seems good.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

sbaugh@catern.com writes:

> Ah, I see.  Then, like pipes and sockets, it should really at least
> support interrupting the transfer...

It's 30 years too late for changes to the ICCCM.  So we will have to
make the best out of what we have.

> Such costs only need to be paid if there are indeed multiple conversions
> happening at the same time.  In the common case there's just one, so
> there's no extra cost of adding the ability to have multiple ongoing
> conversions.

Adding extra round trip cost to a part of Emacs that is already one of
the slowest and most synchronous components of the X Windows support is
unacceptable in my book.  Especially for such an uncommon situation: I
can't remember the last time I quit a selection request to a working
selection owner, and I use Emacs over wide area networks every day.

> Actually, how does this work today?  If an Emacs user quits a conversion
> and then immediately starts a new one, that seems to work fine.  We
> don't do different properties in each request.  I realize the protocol
> doesn't support it, but doesn't that suggest that it's fine in practice
> to interrupt a conversion...?

If Emacs quits while waiting for a property change to take place, and
the property change event from the recipient of the request that was
quit still arrives, Emacs will become confused and return outdated
selection data.  Or even worse, a mixture of the selection data from
both the new and old owners.

Selection owners that are not implemented robustly may also freeze if
Emacs quits before removing the selection transfer property from its
window to acknowledge the selection data's arrival.

> (Quitting a conversion then running another would be one way to have
> multiple conversions at the same time.  Another way would be (related to
> my other thread about call-process) to allow running Lisp while an
> incremental selection transfer is ongoing.  I know that seems useless
> but I really am of the opinion that every blocking operation in Emacs
> which can take more than a few milliseconds should support running Lisp
> while it blocks...)

No, it's only necessary for us to ensure that C-g works, so the user can
always quit from the long-running operation.  Otherwise, your position
cannot be consistent without insisting that constructs such as `while'
also check for and run timers and selection converters.

> Yes.  Personally I'd like to have a lower value for that when a
> save-interprogram-paste-before-kill is triggered.  So adding a separate
> save-interprogram-paste-before-kill-timeout which can be lower, and
> which let-binds x-selection-timeout, seems good.

How is that different from setting `x-selection-timeout'?  IOW, what's
your real-world use case for such an additional option?  Have you tried
adjusting `x-selection-timeout' for all selection transfers?

Let's not overengineer the system independent interprogram code with
X-specific options.  Thanks.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[sbaugh]

Po Lu <luangruo@yahoo.com> writes:
>> Such costs only need to be paid if there are indeed multiple conversions
>> happening at the same time.  In the common case there's just one, so
>> there's no extra cost of adding the ability to have multiple ongoing
>> conversions.
>
> Adding extra round trip cost to a part of Emacs that is already one of
> the slowest and most synchronous components of the X Windows support is
> unacceptable in my book.  Especially for such an uncommon situation: I
> can't remember the last time I quit a selection request to a working
> selection owner, and I use Emacs over wide area networks every day.

I think you might be misunderstanding me?  Just to say again, in the
normal case, this would not add extra cost.  This would only add extra
round trip cost in cases which, as you say below, currently just confuse
and break Emacs.

(Also, the entire point would be that this component would move from
being synchronous to being able to run in the background, concurrent
with other things.)

>> Actually, how does this work today?  If an Emacs user quits a conversion
>> and then immediately starts a new one, that seems to work fine.  We
>> don't do different properties in each request.  I realize the protocol
>> doesn't support it, but doesn't that suggest that it's fine in practice
>> to interrupt a conversion...?
>
> If Emacs quits while waiting for a property change to take place, and
> the property change event from the recipient of the request that was
> quit still arrives, Emacs will become confused and return outdated
> selection data.  Or even worse, a mixture of the selection data from
> both the new and old owners.
>
> Selection owners that are not implemented robustly may also freeze if
> Emacs quits before removing the selection transfer property from its
> window to acknowledge the selection data's arrival.

OK, interesting.  So it's basically broken already today to interrupt a
gui-get-selection... great.

>> (Quitting a conversion then running another would be one way to have
>> multiple conversions at the same time.  Another way would be (related to
>> my other thread about call-process) to allow running Lisp while an
>> incremental selection transfer is ongoing.  I know that seems useless
>> but I really am of the opinion that every blocking operation in Emacs
>> which can take more than a few milliseconds should support running Lisp
>> while it blocks...)
>
> No, it's only necessary for us to ensure that C-g works, so the user can
> always quit from the long-running operation.  Otherwise, your position
> cannot be consistent without insisting that constructs such as `while'
> also check for and run timers and selection converters.

This is becoming tangential, but, yes, I will bite that bullet.  "while"
should also check for and run timers and selection converters, when Lisp
code opts-in to that behavior.  I can think of several ways to implement
this without hurting performance.

IMO the major issue with the Emacs UI at the moment is that it blocks
too much, relative to "modern" applications.  Some of this blocking can
only be fixed by speeding up Lisp execution, but substantial parts of
this blocking can only be fixed by making Emacs more concurrent - that
is, making it possible for Lisp code to run concurrently with other Lisp
code, on an opt-in basis, instead of blocking all Lisp execution while
operations like gui-get-selection and call-process are running.

I am aware this is a major undertaking, but I think it's important for
Emacs to compare favorably with "modern" applications which don't
exhibit as much random UI blocking.  I regard this as basically
equivalent to the lexical-binding transition.

>> Yes.  Personally I'd like to have a lower value for that when a
>> save-interprogram-paste-before-kill is triggered.  So adding a separate
>> save-interprogram-paste-before-kill-timeout which can be lower, and
>> which let-binds x-selection-timeout, seems good.
>
> How is that different from setting `x-selection-timeout'?  IOW, what's
> your real-world use case for such an additional option?  Have you tried
> adjusting `x-selection-timeout' for all selection transfers?

Here is the real-world use case: When I yank I'm willing to wait for 2
or 10 seconds for the paste to complete, since the paste is what I
actually want.  But when I kill-new I want to wait less time, because
the save-interprogram-paste-before-kill behavior is just a nice-to-have,
which I want to happen if it's convenient and fast, not the actual thing
I want to do.

> Let's not overengineer the system independent interprogram code with
> X-specific options.  Thanks.

A gui-selection-timeout would be fine with me, too.  Maybe other systems
would benefit?  pgtk?

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> Cc: Spencer Baugh <sbaugh@janestreet.com>, 64423@debbugs.gnu.org
> From: sbaugh@catern.com
> Date: Tue, 04 Jul 2023 14:46:36 +0000 (UTC)
> 
> IMO the major issue with the Emacs UI at the moment is that it blocks
> too much, relative to "modern" applications.

That is true, but it cannot be fixed by small half-measures.  The
basic Emacs design _assumes_ this single-threaded operation, and many
of the low-level parts will simply break if the assumption becomes
false.  Some will break visibly and loudly, some will break subtly and
silently, but they _will_ break.  We have seen this many times: the
seemingly-confusing code which does things no one completely
understands turns out to do all that for good reasons, which only
become visible when we in our arrogance boldly make changes no one
imagined when this was designed and implemented.  And people who
designed and implemented it, and improved it over the years, were and
are very clever, and knew what they were doing and why.

The _only_ sane way of getting a non-blocking Emacs is to redesign all
of Emacs around that idea.

> Some of this blocking can
> only be fixed by speeding up Lisp execution, but substantial parts of
> this blocking can only be fixed by making Emacs more concurrent - that
> is, making it possible for Lisp code to run concurrently with other Lisp
> code, on an opt-in basis, instead of blocking all Lisp execution while
> operations like gui-get-selection and call-process are running.

You cannot "make Emacs more concurrent", not by and large.  We can
make small improvements here and there, if we tread cautiously and do
careful damage control after each such change, but that's all.  If you
look at the low-level code in Emacs and take time to understand how it
works, you will agree with me.  (And if you don't agree, we will have
this very argument again many times in the future.)  We should accept
that fact, and either live with it or start a new-generation Emacs,
based on very different designs.  Anything else is just lying to
ourselves.

> I am aware this is a major undertaking, but I think it's important for
> Emacs to compare favorably with "modern" applications which don't
> exhibit as much random UI blocking.  I regard this as basically
> equivalent to the lexical-binding transition.

Ha!  Lexical-binding transition is nothing near what you propose.  It
changed the language and its interpreter, but not the editor and its
infrastructure and primitives.  What you suggest is several orders of
magnitude harder (read: impossible).

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

> You cannot "make Emacs more concurrent", not by and large.  We can
> make small improvements here and there, if we tread cautiously and do
> careful damage control after each such change, but that's all.

I feel that I am repeating an already proposed idea, but what about
concurrent isolated process that interacts with the main process?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: sbaugh@catern.com, luangruo@yahoo.com, sbaugh@janestreet.com,
>  64423@debbugs.gnu.org
> Date: Tue, 04 Jul 2023 16:32:31 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > You cannot "make Emacs more concurrent", not by and large.  We can
> > make small improvements here and there, if we tread cautiously and do
> > careful damage control after each such change, but that's all.
> 
> I feel that I am repeating an already proposed idea, but what about
> concurrent isolated process that interacts with the main process?

(This stuff should not be discussed on the bug tracker, but on
emacs-devel.)

If you mean what the async package does, then yes, this is a workable
idea.  But it doesn't need to change anything in Emacs, and it has
some downsides, like the difficulties in sharing state with the other
process.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[sbaugh]

Eli Zaretskii <eliz@gnu.org> writes:
>> Cc: Spencer Baugh <sbaugh@janestreet.com>, 64423@debbugs.gnu.org
>> From: sbaugh@catern.com
>> Date: Tue, 04 Jul 2023 14:46:36 +0000 (UTC)
>> 
>> IMO the major issue with the Emacs UI at the moment is that it blocks
>> too much, relative to "modern" applications.
>
> That is true, but it cannot be fixed by small half-measures.  The
> basic Emacs design _assumes_ this single-threaded operation, and many
> of the low-level parts will simply break if the assumption becomes
> false.  Some will break visibly and loudly, some will break subtly and
> silently, but they _will_ break.  We have seen this many times: the
> seemingly-confusing code which does things no one completely
> understands turns out to do all that for good reasons, which only
> become visible when we in our arrogance boldly make changes no one
> imagined when this was designed and implemented.  And people who
> designed and implemented it, and improved it over the years, were and
> are very clever, and knew what they were doing and why.
>
> The _only_ sane way of getting a non-blocking Emacs is to redesign all
> of Emacs around that idea.
>
>> Some of this blocking can
>> only be fixed by speeding up Lisp execution, but substantial parts of
>> this blocking can only be fixed by making Emacs more concurrent - that
>> is, making it possible for Lisp code to run concurrently with other Lisp
>> code, on an opt-in basis, instead of blocking all Lisp execution while
>> operations like gui-get-selection and call-process are running.
>
> You cannot "make Emacs more concurrent", not by and large.  We can
> make small improvements here and there, if we tread cautiously and do
> careful damage control after each such change, but that's all.

That's all I ask for, the ability to make such small, careful, cautious
improvements.  I think with enough of these, we will get to a much
better place.  As long as every individual small improvement is not
rejected just because it is too small of an improvement...

And I have the ability to test these improvements at my site (of ~500
users with diverse configurations), so I am in a decent position to make
these small improvements without breaking Emacs.  As long as I don't
have to carry those patches forever...

> If you look at the low-level code in Emacs and take time to understand
> how it works, you will agree with me.  (And if you don't agree, we
> will have this very argument again many times in the future.)  We
> should accept that fact, and either live with it or start a
> new-generation Emacs, based on very different designs.  Anything else
> is just lying to ourselves.

I've stared at low-level code in Emacs a good amount, but certainly I
wouldn't claim to understand Emacs yet.  Nevertheless...

I think Lisp threads shows that this is not true.  With that as the
foundation, and making incremental small improvements to the C core to
increase the amount of stuff that can be done in Lisp threads, we can
build a new-generation Emacs incrementally within the old.  IMO the
reason we haven't done this yet because Lisp threads are still very
limited in what they can do without blocking.

But we don't have to agree - I'm happy to make small, step-by-step
improvements, which are justifiable on their own.  That's what I'm doing
now: I am trying to fix blocking issues which I get lots of user
complaints about, not change things for no reason.

>> I am aware this is a major undertaking, but I think it's important for
>> Emacs to compare favorably with "modern" applications which don't
>> exhibit as much random UI blocking.  I regard this as basically
>> equivalent to the lexical-binding transition.
>
> Ha!  Lexical-binding transition is nothing near what you propose.  It
> changed the language and its interpreter, but not the editor and its
> infrastructure and primitives.  What you suggest is several orders of
> magnitude harder (read: impossible).

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Tue, 04 Jul 2023 16:48:06 +0000 (UTC)
> Cc: luangruo@yahoo.com, sbaugh@janestreet.com, 64423@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > You cannot "make Emacs more concurrent", not by and large.  We can
> > make small improvements here and there, if we tread cautiously and do
> > careful damage control after each such change, but that's all.
> 
> That's all I ask for, the ability to make such small, careful, cautious
> improvements.

We seem to differ in what we call "small" improvements...

> > If you look at the low-level code in Emacs and take time to understand
> > how it works, you will agree with me.  (And if you don't agree, we
> > will have this very argument again many times in the future.)  We
> > should accept that fact, and either live with it or start a
> > new-generation Emacs, based on very different designs.  Anything else
> > is just lying to ourselves.
> 
> I've stared at low-level code in Emacs a good amount, but certainly I
> wouldn't claim to understand Emacs yet.  Nevertheless...
> 
> I think Lisp threads shows that this is not true.

If what I said were indeed not true, Lisp threads would have been a
hot feature, used by every package out there.  That it is not so
should teach us something.  I don't know if you tried to write a
serious application based on Lisp threads, but if not, maybe you
should try.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> I think Lisp threads shows that this is not true.
>
> If what I said were indeed not true, Lisp threads would have been a
> hot feature, used by every package out there.  That it is not so
> should teach us something.  I don't know if you tried to write a
> serious application based on Lisp threads, but if not, maybe you
> should try.

IMHO, the main problem with threads is that they cannot be "paused" at
arbitrary moment. So, without sprinkling too many `thread-yield', any
serious computation makes the main command loop unusable.

Org mode has asynchronous processing since before threads were
introduced, using idle timers. It works seamlessly, but only thanks to
carefully balanced `org-element-cache-sync-idle-time',
`org-element-cache-sync-duration', and `org-element-cache-sync-break'
that limit how aggressively the background async computation is fired.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: sbaugh@catern.com, luangruo@yahoo.com, sbaugh@janestreet.com,
>  64423@debbugs.gnu.org
> Date: Tue, 04 Jul 2023 17:14:22 +0000
> 
> Org mode has asynchronous processing since before threads were
> introduced, using idle timers.

That timers in Emacs work is small wonder.

The point of threads, AFAIU, was to allow doing the same stuff as we
do with timers, but with significantly less hair on the Lisp level.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> Org mode has asynchronous processing since before threads were
>> introduced, using idle timers.
>
> That timers in Emacs work is small wonder.
>
> The point of threads, AFAIU, was to allow doing the same stuff as we
> do with timers, but with significantly less hair on the Lisp level.

Yup. That's my point - thread can be useful, especially if there is also
some boilerplate code provided by Emacs that makes threads less blocking
(read: trigger less frequently, and stopping not after the user attempts
to trigger command loop).

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

sbaugh@catern.com writes:

> I think you might be misunderstanding me?  Just to say again, in the
> normal case, this would not add extra cost.  This would only add extra
> round trip cost in cases which, as you say below, currently just confuse
> and break Emacs.

It is extra cost, for an insignificant problem whose fix is unwarranted.

> (Also, the entire point would be that this component would move from
> being synchronous to being able to run in the background, concurrent
> with other things.)

No, setting up the selection transfer will still remain synchronous, and
x-get-selection-internal will continue to block.

> This is becoming tangential, but, yes, I will bite that bullet.  "while"
> should also check for and run timers and selection converters, when Lisp
> code opts-in to that behavior.  I can think of several ways to implement
> this without hurting performance.

That's unsafe, and that's simply not how Emacs works.  You're talking
about turning code utilizing while into signal handlers with strict
reentrancy requirements.

> IMO the major issue with the Emacs UI at the moment is that it blocks
> too much, relative to "modern" applications.  Some of this blocking can
> only be fixed by speeding up Lisp execution, but substantial parts of
> this blocking can only be fixed by making Emacs more concurrent - that
> is, making it possible for Lisp code to run concurrently with other Lisp
> code, on an opt-in basis, instead of blocking all Lisp execution while
> operations like gui-get-selection and call-process are running.

Other UI toolkits also block waiting for selection data to arrive.  They
even block when responding to selection requests, while Emacs can
respond to multiple outstanding selection requests simultaneously.

> Here is the real-world use case: When I yank I'm willing to wait for 2
> or 10 seconds for the paste to complete, since the paste is what I
> actually want.  But when I kill-new I want to wait less time, because
> the save-interprogram-paste-before-kill behavior is just a nice-to-have,
> which I want to happen if it's convenient and fast, not the actual thing
> I want to do.

Have you actually tried setting just `x-selection-timeout' and found it
unsatisfactory?

>> Let's not overengineer the system independent interprogram code with
>> X-specific options.  Thanks.
>
> A gui-selection-timeout would be fine with me, too.  Maybe other systems
> would benefit?  pgtk?

No other window systems will benefit from this arrangement, because
their selection transfer mechanisms are not interruptable.

`pgtk-selection-timeout' exists as lip service to the GDK selection
mechanism and doesn't actually work on Wayland.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> That timers in Emacs work is small wonder.
>
> The point of threads, AFAIU, was to allow doing the same stuff as we
> do with timers, but with significantly less hair on the Lisp level.

IIRC idle timers used to be implemented in Lisp, with an asynchronous
process sending output whenever a timer expired :-)

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Ihor Radchenko <yantar92@posteo.net>,  sbaugh@catern.com,
>   sbaugh@janestreet.com,  64423@debbugs.gnu.org
> Date: Wed, 05 Jul 2023 08:30:12 +0800
> 
> IIRC idle timers used to be implemented in Lisp, with an asynchronous
> process sending output whenever a timer expired :-)

No, _all_ timers were implemented like that at the beginning, before
they were rewritten based on the Emacs main loop.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> No, _all_ timers were implemented like that at the beginning, before
> they were rewritten based on the Emacs main loop.

That's what I meant to say, thanks.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: yantar92@posteo.net,  sbaugh@catern.com,  sbaugh@janestreet.com,
>   64423@debbugs.gnu.org
> Date: Wed, 05 Jul 2023 11:51:30 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > No, _all_ timers were implemented like that at the beginning, before
> > they were rewritten based on the Emacs main loop.
> 
> That's what I meant to say, thanks.

Also, FTR: that special subprocess didn't produce any output; instead,
it delivered SIGALRM to Emacs.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Spencer Baugh]

Po Lu <luangruo@yahoo.com> writes:

> sbaugh@catern.com writes:
>
>> I think you might be misunderstanding me?  Just to say again, in the
>> normal case, this would not add extra cost.  This would only add extra
>> round trip cost in cases which, as you say below, currently just confuse
>> and break Emacs.
>
> It is extra cost, for an insignificant problem whose fix is unwarranted.

The insignificant problem is Emacs potentially getting the wrong
selection if we interrupt an incremental selection transfer?

I'm confused, it seems like that contradicts what you said earlier.

If interrupting an incremental selection transfer is an insignificant
problem, then there should be no obstacle to automatically interrupting
the incremental selection transfer if it's too large.

>> (Also, the entire point would be that this component would move from
>> being synchronous to being able to run in the background, concurrent
>> with other things.)
>
> No, setting up the selection transfer will still remain synchronous, and
> x-get-selection-internal will continue to block.
>
>> This is becoming tangential, but, yes, I will bite that bullet.  "while"
>> should also check for and run timers and selection converters, when Lisp
>> code opts-in to that behavior.  I can think of several ways to implement
>> this without hurting performance.
>
> That's unsafe, and that's simply not how Emacs works.  You're talking
> about turning code utilizing while into signal handlers with strict
> reentrancy requirements.
>
>> IMO the major issue with the Emacs UI at the moment is that it blocks
>> too much, relative to "modern" applications.  Some of this blocking can
>> only be fixed by speeding up Lisp execution, but substantial parts of
>> this blocking can only be fixed by making Emacs more concurrent - that
>> is, making it possible for Lisp code to run concurrently with other Lisp
>> code, on an opt-in basis, instead of blocking all Lisp execution while
>> operations like gui-get-selection and call-process are running.
>
> Other UI toolkits also block waiting for selection data to arrive.  They
> even block when responding to selection requests, while Emacs can
> respond to multiple outstanding selection requests simultaneously.

OK, so we are doing at least as good as other toolkits when it comes to
retrieving the selection.  But at my site the UX is still worse than
other applications because save-interprogram-paste-before-kill makes
taking ownership of the selection block, while for other applications it
does not.  And save-interprogram-paste-before-kill is a useful feature,
and I want to make it work.

>> Here is the real-world use case: When I yank I'm willing to wait for 2
>> or 10 seconds for the paste to complete, since the paste is what I
>> actually want.  But when I kill-new I want to wait less time, because
>> the save-interprogram-paste-before-kill behavior is just a nice-to-have,
>> which I want to happen if it's convenient and fast, not the actual thing
>> I want to do.
>
> Have you actually tried setting just `x-selection-timeout' and found it
> unsatisfactory?

Yes.  x-selection-timeout is configured to 5 seconds for every user at
my site.  They still find it unexpected and complain when killing takes
that long.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Spencer Baugh <sbaugh@janestreet.com> writes:

> The insignificant problem is Emacs potentially getting the wrong
> selection if we interrupt an incremental selection transfer?

Yes, when the user does so deliberately.

> I'm confused, it seems like that contradicts what you said earlier.
>
> If interrupting an incremental selection transfer is an insignificant
> problem, then there should be no obstacle to automatically interrupting
> the incremental selection transfer if it's too large.

Interrupting incremental selection transfer _automatically_ is a
significant problem, because Emacs cannot judge if the selection owner
is functioning normally.  If the user quits, he has determined that it
is not, so it is acceptable for Emacs to terminate the transfer there
and then without considering the consequences to the selection owner and
future selection transfers.

> OK, so we are doing at least as good as other toolkits when it comes to
> retrieving the selection.  But at my site the UX is still worse than
> other applications because save-interprogram-paste-before-kill makes
> taking ownership of the selection block, while for other applications it
> does not.  And save-interprogram-paste-before-kill is a useful feature,
> and I want to make it work.

Other programs do not have a kill ring at all.

> Yes.  x-selection-timeout is configured to 5 seconds for every user at
> my site.  They still find it unexpected and complain when killing takes
> that long.

But do they complain about inserting the contents of the selection also
taking too long?  Or when a program other than Emacs blocks for more
than 5 seconds upon Button2 or Ctrl+V?

Anyway, this points to a problem with an X client at your site and not a
problem with Emacs.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Spencer Baugh]

Po Lu <luangruo@yahoo.com> writes:
> Spencer Baugh <sbaugh@janestreet.com> writes:
>
>> The insignificant problem is Emacs potentially getting the wrong
>> selection if we interrupt an incremental selection transfer?
>
> Yes, when the user does so deliberately.
>
>> I'm confused, it seems like that contradicts what you said earlier.
>>
>> If interrupting an incremental selection transfer is an insignificant
>> problem, then there should be no obstacle to automatically interrupting
>> the incremental selection transfer if it's too large.
>
> Interrupting incremental selection transfer _automatically_ is a
> significant problem, because Emacs cannot judge if the selection owner
> is functioning normally.  If the user quits, he has determined that it
> is not, so it is acceptable for Emacs to terminate the transfer there
> and then without considering the consequences to the selection owner and
> future selection transfers.

And yet, we do this today: that's what x-selection-timeout does.  Should
we remove that functionality?

I assume we should not remove that functionality.  So if automatically
interrupting a selection transfer if the owner takes too long is fine,
what's the issue with interrupting it if the owner sends too much data?
Both situations are usually the result of buggy X clients, both
situations would break Emacs if not handled, both situations are
standard considerations for robustness in any network protocol.

>> OK, so we are doing at least as good as other toolkits when it comes to
>> retrieving the selection.  But at my site the UX is still worse than
>> other applications because save-interprogram-paste-before-kill makes
>> taking ownership of the selection block, while for other applications it
>> does not.  And save-interprogram-paste-before-kill is a useful feature,
>> and I want to make it work.
>
> Other programs do not have a kill ring at all.

Yes.  And Emacs does, so it needs to do more work than other
applications to make to work correctly.

>> Yes.  x-selection-timeout is configured to 5 seconds for every user at
>> my site.  They still find it unexpected and complain when killing takes
>> that long.
>
> But do they complain about inserting the contents of the selection
> also taking too long?  Or when a program other than Emacs blocks for
> more than 5 seconds upon Button2 or Ctrl+V?

No, because then they are performing an operation which it makes sense
might block: pasting data copied from another application.  In that
situation, they are fine with it.

> Anyway, this points to a problem with an X client at your site and not a
> problem with Emacs.

No, there is no problem with other X clients.  It is simply that users
expect delays when yanking and don't expect delays when killing.

So, Emacs should be able to configure a different x-selection-timeout
when running the save-interprogram-paste-before-kill logic, to reflect
the fact that users have these different expectations for yanking and
killing.  I don't see why this is objectionable.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Spencer Baugh <sbaugh@janestreet.com> writes:

> And yet, we do this today: that's what x-selection-timeout does.  Should
> we remove that functionality?

[...]

> I assume we should not remove that functionality.  So if automatically
> interrupting a selection transfer if the owner takes too long is fine,
> what's the issue with interrupting it if the owner sends too much data?

Because sending a lot of data is *NOT* a bug in the selection owner.
Delaying subsequent user activity for a significant amount of time is.

> Both situations are usually the result of buggy X clients, both
> situations would break Emacs if not handled, both situations are
> standard considerations for robustness in any network protocol.

Blocking user interaction while quitting remains possible is not
``broken'' in my book.

> No, because then they are performing an operation which it makes sense
> might block: pasting data copied from another application.  In that
> situation, they are fine with it.

[...]

> No, there is no problem with other X clients.  It is simply that users
> expect delays when yanking and don't expect delays when killing.
>
> So, Emacs should be able to configure a different x-selection-timeout
> when running the save-interprogram-paste-before-kill logic, to reflect
> the fact that users have these different expectations for yanking and
> killing.  I don't see why this is objectionable.

Taking more than five seconds to yank points to a bug in whichever
client is owning the clipboard selection at the time of the yank.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[sbaugh]

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

sbaugh@catern.com writes:
> Po Lu <luangruo@yahoo.com> writes:
>> sbaugh@catern.com writes:
>>
>>> Po Lu <luangruo@yahoo.com> writes:
>>>
>>>> Spencer Baugh <sbaugh@janestreet.com> writes:
>>>>
>>>>> When you do that, you interrupt the operation which is trying to add a
>>>>> new kill.  If you interrupt it and try again, you'll just get the same
>>>>> long delay again.  There's no way to mitigate this from within Emacs,
>>>>> other than by turning off save-interprogram-paste-before-kill.
>>>>
>>>> Then I guess the solution is to temporarily disable
>>>> `save-interprogram-paste-before-kill' if a quit arrives while it is
>>>> reading selection data.
>>>
>>> That would be a decent solution.  Although I'm not sure how we'd
>>> implement it.  We want to, somehow, know that after a selection-transfer
>>> has been aborted, we should not try to transfer that selection again.
>>> Is that something we can check?  Whether the selection has changed,
>>> without transferring it?
>>
>> Emacs will probably assert ownership of the selection after the kill
>> takes place, anyway, so there is no need.
>
> Good point!  So maybe if a quit arrives while we're reading the
> selection in kill-new, we should immediately take ownership of the
> selection.  With an condition-case, perhaps.
>
> Or... just ignore the quit.  If we're reading the selection in kill-new
> and there's a quit, just proceed to doing the kill.

Here is an implementation of that.  I think this is the right strategy
and I'm glad we discussed this, I think this will behave better for
users than my original suggestion, and this way doesn't require any
configuration.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Ignore-quit-while-getting-interprogram-paste-in-kill.patch --]
[-- Type: text/x-patch, Size: 2107 bytes --]

From b7b0feb879994696bdd6f6f4cc982779b1ff5b45 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@catern.com>
Date: Sat, 8 Jul 2023 12:36:22 -0400
Subject: [PATCH] Ignore quit while getting interprogram paste in kill-new

On X, if the current selection owner is not responding to selection
requests, the user may want to take ownership of the selection.  The
obvious way to do this is to kill some text (which a user might also
be doing just as part of normal editing at the time the selection
owner becomes nonresponsive).  However, if
save-interprogram-paste-before-kill is non-nil, then killing text will
hang until the user quits, and this quit will abort the entire
kill-new, preventing the user from taking ownership of the selection.

Now instead if the user quits while we are attempting to retrieve the
selection from hanging owner, we will proceed to take ownership of the
selection as normal, resolving the problem.

(One example of a selction owner that might not be responding to
selection requests is another instance of Emacs itself; while Emacs is
blocked in call-process or Lisp execution, it currently does not
respond to selection requests.)

* lisp/simple.el (kill-new): Ignore quit while getting interprogram
paste (bug#64423)
---
 lisp/simple.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index 26944f1f72d..95d00cc506b 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -5618,8 +5618,10 @@ kill-new
       (if (fboundp 'menu-bar-update-yank-menu)
 	  (menu-bar-update-yank-menu string (and replace (car kill-ring)))))
     (when save-interprogram-paste-before-kill
-      (let ((interprogram-paste (and interprogram-paste-function
-                                     (funcall interprogram-paste-function))))
+      (let ((interprogram-paste
+             (ignore-error 'quit
+               (and interprogram-paste-function
+                    (funcall interprogram-paste-function)))))
         (when interprogram-paste
           (setq interprogram-paste
                 (if (listp interprogram-paste)
-- 
2.41.0

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> Cc: Spencer Baugh <sbaugh@janestreet.com>, 64423@debbugs.gnu.org
> From: sbaugh@catern.com
> Date: Sat, 08 Jul 2023 16:39:00 +0000 (UTC)
> 
> diff --git a/lisp/simple.el b/lisp/simple.el
> index 26944f1f72d..95d00cc506b 100644
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -5618,8 +5618,10 @@ kill-new
>        (if (fboundp 'menu-bar-update-yank-menu)
>  	  (menu-bar-update-yank-menu string (and replace (car kill-ring)))))
>      (when save-interprogram-paste-before-kill
> -      (let ((interprogram-paste (and interprogram-paste-function
> -                                     (funcall interprogram-paste-function))))
> +      (let ((interprogram-paste
> +             (ignore-error 'quit
> +               (and interprogram-paste-function
> +                    (funcall interprogram-paste-function)))))
>          (when interprogram-paste
>            (setq interprogram-paste
>                  (if (listp interprogram-paste)

Are you sure this is TRT for all the implementations of GUI
selections?  AFAIU, the discussion was only about X.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:
>> Cc: Spencer Baugh <sbaugh@janestreet.com>, 64423@debbugs.gnu.org
>> From: sbaugh@catern.com
>> Date: Sat, 08 Jul 2023 16:39:00 +0000 (UTC)
>> 
>> diff --git a/lisp/simple.el b/lisp/simple.el
>> index 26944f1f72d..95d00cc506b 100644
>> --- a/lisp/simple.el
>> +++ b/lisp/simple.el
>> @@ -5618,8 +5618,10 @@ kill-new
>>        (if (fboundp 'menu-bar-update-yank-menu)
>>  	  (menu-bar-update-yank-menu string (and replace (car kill-ring)))))
>>      (when save-interprogram-paste-before-kill
>> -      (let ((interprogram-paste (and interprogram-paste-function
>> -                                     (funcall interprogram-paste-function))))
>> +      (let ((interprogram-paste
>> +             (ignore-error 'quit
>> +               (and interprogram-paste-function
>> +                    (funcall interprogram-paste-function)))))
>>          (when interprogram-paste
>>            (setq interprogram-paste
>>                  (if (listp interprogram-paste)
>
> Are you sure this is TRT for all the implementations of GUI
> selections?  AFAIU, the discussion was only about X.

Independent of that discussion, I think this change should be harmless.
The worst thing that this change can cause is that a call to kill-new
can complete successfully when it otherwise would have failed.  That's
probably always good, especially because kill-new is usually preceeded
by deleting text, which might be unrecoverable in buffers without undo.

But, also, I believe the discussion makes sense for platforms besides X:
if there's a bug in the current owner of the clipboard, then taking
ownership of the clipboard in Emacs will let us avoid that bug.  And
this change to ignore quits will allow the user to take ownership of the
selection in that way, whereas they previously would not be able to
(without manually writing some Lisp anyway).

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: sbaugh@catern.com,  luangruo@yahoo.com,  64423@debbugs.gnu.org
> Date: Sat, 08 Jul 2023 13:07:37 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> >> Cc: Spencer Baugh <sbaugh@janestreet.com>, 64423@debbugs.gnu.org
> >> From: sbaugh@catern.com
> >> Date: Sat, 08 Jul 2023 16:39:00 +0000 (UTC)
> >> 
> >> diff --git a/lisp/simple.el b/lisp/simple.el
> >> index 26944f1f72d..95d00cc506b 100644
> >> --- a/lisp/simple.el
> >> +++ b/lisp/simple.el
> >> @@ -5618,8 +5618,10 @@ kill-new
> >>        (if (fboundp 'menu-bar-update-yank-menu)
> >>  	  (menu-bar-update-yank-menu string (and replace (car kill-ring)))))
> >>      (when save-interprogram-paste-before-kill
> >> -      (let ((interprogram-paste (and interprogram-paste-function
> >> -                                     (funcall interprogram-paste-function))))
> >> +      (let ((interprogram-paste
> >> +             (ignore-error 'quit
> >> +               (and interprogram-paste-function
> >> +                    (funcall interprogram-paste-function)))))
> >>          (when interprogram-paste
> >>            (setq interprogram-paste
> >>                  (if (listp interprogram-paste)
> >
> > Are you sure this is TRT for all the implementations of GUI
> > selections?  AFAIU, the discussion was only about X.
> 
> Independent of that discussion, I think this change should be harmless.

How do you know that?  The prudent thing in Emacs is to "do no harm",
i.e. try hard not to affect any code that is unrelated to the problem.
Assuming that a change is harmless is a mother of all bugs.

> The worst thing that this change can cause is that a call to kill-new
> can complete successfully when it otherwise would have failed.

No, I think you can also do the reverse.  And anyway, this kind of
"reasoning" is what gets us in trouble time and again.  Why risk all
those potential problems, where the original issue doesn't exist in
the first place?

> But, also, I believe the discussion makes sense for platforms besides X:
> if there's a bug in the current owner of the clipboard, then taking
> ownership of the clipboard in Emacs will let us avoid that bug.  And
> this change to ignore quits will allow the user to take ownership of the
> selection in that way, whereas they previously would not be able to
> (without manually writing some Lisp anyway).

You do realize that some window systems Emacs support have no notion
of "clipboard ownership" or "selection ownership" whatsoever?

So please modify the patch to affect only X, TIA.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Cc: sbaugh@catern.com,  luangruo@yahoo.com,  64423@debbugs.gnu.org
>> Date: Sat, 08 Jul 2023 13:07:37 -0400
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> >> Cc: Spencer Baugh <sbaugh@janestreet.com>, 64423@debbugs.gnu.org
>> >> From: sbaugh@catern.com
>> >> Date: Sat, 08 Jul 2023 16:39:00 +0000 (UTC)
>> >> 
>> >> diff --git a/lisp/simple.el b/lisp/simple.el
>> >> index 26944f1f72d..95d00cc506b 100644
>> >> --- a/lisp/simple.el
>> >> +++ b/lisp/simple.el
>> >> @@ -5618,8 +5618,10 @@ kill-new
>> >>        (if (fboundp 'menu-bar-update-yank-menu)
>> >>  	  (menu-bar-update-yank-menu string (and replace (car kill-ring)))))
>> >>      (when save-interprogram-paste-before-kill
>> >> -      (let ((interprogram-paste (and interprogram-paste-function
>> >> -                                     (funcall interprogram-paste-function))))
>> >> +      (let ((interprogram-paste
>> >> +             (ignore-error 'quit
>> >> +               (and interprogram-paste-function
>> >> +                    (funcall interprogram-paste-function)))))
>> >>          (when interprogram-paste
>> >>            (setq interprogram-paste
>> >>                  (if (listp interprogram-paste)
>> >
>> > Are you sure this is TRT for all the implementations of GUI
>> > selections?  AFAIU, the discussion was only about X.
>> 
>> Independent of that discussion, I think this change should be harmless.
>
> How do you know that?  The prudent thing in Emacs is to "do no harm",
> i.e. try hard not to affect any code that is unrelated to the problem.
> Assuming that a change is harmless is a mother of all bugs.
>
>> The worst thing that this change can cause is that a call to kill-new
>> can complete successfully when it otherwise would have failed.
>
> No, I think you can also do the reverse.  And anyway, this kind of
> "reasoning" is what gets us in trouble time and again.  Why risk all
> those potential problems, where the original issue doesn't exist in
> the first place?
>
>> But, also, I believe the discussion makes sense for platforms besides X:
>> if there's a bug in the current owner of the clipboard, then taking
>> ownership of the clipboard in Emacs will let us avoid that bug.  And
>> this change to ignore quits will allow the user to take ownership of the
>> selection in that way, whereas they previously would not be able to
>> (without manually writing some Lisp anyway).
>
> You do realize that some window systems Emacs support have no notion
> of "clipboard ownership" or "selection ownership" whatsoever?
>
> So please modify the patch to affect only X, TIA.

It's impossible to quit from selection transfers in other window systems
anyway, except perhaps PGTK when the toolkit is feeling generous.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Spencer Baugh <sbaugh@janestreet.com>,  sbaugh@catern.com,
>   64423@debbugs.gnu.org
> Date: Sun, 09 Jul 2023 08:39:51 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > So please modify the patch to affect only X, TIA.
> 
> It's impossible to quit from selection transfers in other window systems
> anyway, except perhaps PGTK when the toolkit is feeling generous.

Why "impossible"?  Part of the processing is in Lisp, so it is
definitely possible.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Why "impossible"?  Part of the processing is in Lisp, so it is
> definitely possible.

That processing is also performed under X.  Anyway, I see no need to
disable this code on other window systems.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: sbaugh@janestreet.com,  sbaugh@catern.com,  64423@debbugs.gnu.org
> Date: Sun, 09 Jul 2023 14:12:42 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Why "impossible"?  Part of the processing is in Lisp, so it is
> > definitely possible.
> 
> That processing is also performed under X.  Anyway, I see no need to
> disable this code on other window systems.

Right, so we agree that a change for this issue should be X-specific.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[sbaugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: sbaugh@janestreet.com,  sbaugh@catern.com,  64423@debbugs.gnu.org
>> Date: Sun, 09 Jul 2023 14:12:42 +0800
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Why "impossible"?  Part of the processing is in Lisp, so it is
>> > definitely possible.
>> 
>> That processing is also performed under X.  Anyway, I see no need to
>> disable this code on other window systems.
>
> Right, so we agree that a change for this issue should be X-specific.

Isn't that the opposite of what he said?  That there's no need to
disable this code (which I assume refers to the code I added) on other
window systems?

(Personally I don't care whether this is disabled on other window
systems or not, but if it is disabled on other window systems I'd rather
do it in a way that is not objectionable to Po Lu)

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

sbaugh@catern.com writes:

> Isn't that the opposite of what he said?  That there's no need to
> disable this code (which I assume refers to the code I added) on other
> window systems?

You are correct.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Wed, 12 Jul 2023 19:18:21 +0000 (UTC)
> Cc: Po Lu <luangruo@yahoo.com>, sbaugh@janestreet.com, 64423@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Po Lu <luangruo@yahoo.com>
> >> Cc: sbaugh@janestreet.com,  sbaugh@catern.com,  64423@debbugs.gnu.org
> >> Date: Sun, 09 Jul 2023 14:12:42 +0800
> >> 
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >> 
> >> > Why "impossible"?  Part of the processing is in Lisp, so it is
> >> > definitely possible.
> >> 
> >> That processing is also performed under X.  Anyway, I see no need to
> >> disable this code on other window systems.
> >
> > Right, so we agree that a change for this issue should be X-specific.
> 
> Isn't that the opposite of what he said?  That there's no need to
> disable this code (which I assume refers to the code I added) on other
> window systems?

If that's what he said, then we don't agree.

> (Personally I don't care whether this is disabled on other window
> systems or not, but if it is disabled on other window systems I'd rather
> do it in a way that is not objectionable to Po Lu)

You can do it in a way that is not objectionable to either of us.  It
is very simple: make the changes conditioned on X.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[sbaugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: sbaugh@catern.com
>> Date: Wed, 12 Jul 2023 19:18:21 +0000 (UTC)
>> Cc: Po Lu <luangruo@yahoo.com>, sbaugh@janestreet.com, 64423@debbugs.gnu.org
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Po Lu <luangruo@yahoo.com>
>> >> Cc: sbaugh@janestreet.com,  sbaugh@catern.com,  64423@debbugs.gnu.org
>> >> Date: Sun, 09 Jul 2023 14:12:42 +0800
>> >> 
>> >> Eli Zaretskii <eliz@gnu.org> writes:
>> >> 
>> >> > Why "impossible"?  Part of the processing is in Lisp, so it is
>> >> > definitely possible.
>> >> 
>> >> That processing is also performed under X.  Anyway, I see no need to
>> >> disable this code on other window systems.
>> >
>> > Right, so we agree that a change for this issue should be X-specific.
>> 
>> Isn't that the opposite of what he said?  That there's no need to
>> disable this code (which I assume refers to the code I added) on other
>> window systems?
>
> If that's what he said, then we don't agree.
>
>> (Personally I don't care whether this is disabled on other window
>> systems or not, but if it is disabled on other window systems I'd rather
>> do it in a way that is not objectionable to Po Lu)
>
> You can do it in a way that is not objectionable to either of us.  It
> is very simple: make the changes conditioned on X.

OK, how about this?

modified   lisp/simple.el
@@ -5618,8 +5618,11 @@ kill-new
       (if (fboundp 'menu-bar-update-yank-menu)
 	  (menu-bar-update-yank-menu string (and replace (car kill-ring)))))
     (when save-interprogram-paste-before-kill
-      (let ((interprogram-paste (and interprogram-paste-function
-                                     (funcall interprogram-paste-function))))
+      (let ((interprogram-paste
+             (and interprogram-paste-function
+                  (if (eq (window-system) 'x)
+                      (ignore-error 'quit (funcall interprogram-paste-function))
+                    (funcall interprogram-paste-function)))))
         (when interprogram-paste
           (setq interprogram-paste
                 (if (listp interprogram-paste)

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Thu, 13 Jul 2023 16:17:32 +0000 (UTC)
> Cc: luangruo@yahoo.com, sbaugh@janestreet.com, 64423@debbugs.gnu.org
> 
> > You can do it in a way that is not objectionable to either of us.  It
> > is very simple: make the changes conditioned on X.
> 
> OK, how about this?
> 
> modified   lisp/simple.el
> @@ -5618,8 +5618,11 @@ kill-new
>        (if (fboundp 'menu-bar-update-yank-menu)
>  	  (menu-bar-update-yank-menu string (and replace (car kill-ring)))))
>      (when save-interprogram-paste-before-kill
> -      (let ((interprogram-paste (and interprogram-paste-function
> -                                     (funcall interprogram-paste-function))))
> +      (let ((interprogram-paste
> +             (and interprogram-paste-function
> +                  (if (eq (window-system) 'x)
> +                      (ignore-error 'quit (funcall interprogram-paste-function))
> +                    (funcall interprogram-paste-function)))))
>          (when interprogram-paste
>            (setq interprogram-paste
>                  (if (listp interprogram-paste)

Fine by me, but please add a comment there explaining why we do that
on X.

Thanks.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[sbaugh]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: sbaugh@catern.com
>> Date: Thu, 13 Jul 2023 16:17:32 +0000 (UTC)
>> Cc: luangruo@yahoo.com, sbaugh@janestreet.com, 64423@debbugs.gnu.org
>> 
>> > You can do it in a way that is not objectionable to either of us.  It
>> > is very simple: make the changes conditioned on X.
>> 
>> OK, how about this?
>> 
>> modified   lisp/simple.el
>> @@ -5618,8 +5618,11 @@ kill-new
>>        (if (fboundp 'menu-bar-update-yank-menu)
>>  	  (menu-bar-update-yank-menu string (and replace (car kill-ring)))))
>>      (when save-interprogram-paste-before-kill
>> -      (let ((interprogram-paste (and interprogram-paste-function
>> -                                     (funcall interprogram-paste-function))))
>> +      (let ((interprogram-paste
>> +             (and interprogram-paste-function
>> +                  (if (eq (window-system) 'x)
>> +                      (ignore-error 'quit (funcall interprogram-paste-function))
>> +                    (funcall interprogram-paste-function)))))
>>          (when interprogram-paste
>>            (setq interprogram-paste
>>                  (if (listp interprogram-paste)
>
> Fine by me, but please add a comment there explaining why we do that
> on X.
>
> Thanks.

OK, comment added, here's the patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Ignore-quit-while-getting-interprogram-paste-in-kill.patch --]
[-- Type: text/x-patch, Size: 2445 bytes --]

From 4d669af4c6273d5c7ca229353c5056e3969f84ae Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@catern.com>
Date: Sat, 8 Jul 2023 12:36:22 -0400
Subject: [PATCH] Ignore quit while getting interprogram paste in kill-new

On X, if the current selection owner is not responding to selection
requests, the user may want to take ownership of the selection.  The
obvious way to do this is to kill some text (which a user might also
be doing just as part of normal editing at the time the selection
owner becomes nonresponsive).  However, if
save-interprogram-paste-before-kill is non-nil, then killing text will
hang until the user quits, and this quit will abort the entire
kill-new, preventing the user from taking ownership of the selection.

Now instead if the user quits while we are attempting to retrieve the
selection from hanging owner, we will proceed to take ownership of the
selection as normal, resolving the problem.

(One example of a selction owner that might not be responding to
selection requests is another instance of Emacs itself; while Emacs is
blocked in call-process or Lisp execution, it currently does not
respond to selection requests.)

* lisp/simple.el (kill-new): Ignore quit while getting interprogram
paste (bug#64423)
---
 lisp/simple.el | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index 26944f1f72d..b97b5dd1725 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -5618,8 +5618,14 @@ kill-new
       (if (fboundp 'menu-bar-update-yank-menu)
 	  (menu-bar-update-yank-menu string (and replace (car kill-ring)))))
     (when save-interprogram-paste-before-kill
-      (let ((interprogram-paste (and interprogram-paste-function
-                                     (funcall interprogram-paste-function))))
+      (let ((interprogram-paste
+             (and interprogram-paste-function
+                  ;; On X, the selection owner might be slow, so the user might
+                  ;; interrupt this. If they interrupt it, we want to continue
+                  ;; so we become selection owner, so this doesn't stay slow.
+                  (if (eq (window-system) 'x)
+                      (ignore-error 'quit (funcall interprogram-paste-function))
+                    (funcall interprogram-paste-function)))))
         (when interprogram-paste
           (setq interprogram-paste
                 (if (listp interprogram-paste)
-- 
2.41.0

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Thu, 13 Jul 2023 22:39:10 +0000 (UTC)
> Cc: luangruo@yahoo.com, sbaugh@janestreet.com, 64423@debbugs.gnu.org
> 
> > Fine by me, but please add a comment there explaining why we do that
> > on X.
> >
> > Thanks.
> 
> OK, comment added, here's the patch.

Thanks, LGTM.  Po Lu, any objections?

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: sbaugh@catern.com
>> Date: Thu, 13 Jul 2023 22:39:10 +0000 (UTC)
>> Cc: luangruo@yahoo.com, sbaugh@janestreet.com, 64423@debbugs.gnu.org
>> 
>> > Fine by me, but please add a comment there explaining why we do
>> > that
>> > on X.
>> >
>> > Thanks.
>> 
>> OK, comment added, here's the patch.
>
> Thanks, LGTM.  Po Lu, any objections?

None here.  Please go ahead and install it.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: sbaugh@catern.com,  sbaugh@janestreet.com,  64423@debbugs.gnu.org
> Date: Sat, 15 Jul 2023 16:33:13 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: sbaugh@catern.com
> >> Date: Thu, 13 Jul 2023 22:39:10 +0000 (UTC)
> >> Cc: luangruo@yahoo.com, sbaugh@janestreet.com, 64423@debbugs.gnu.org
> >> 
> >> > Fine by me, but please add a comment there explaining why we do
> >> > that
> >> > on X.
> >> >
> >> > Thanks.
> >> 
> >> OK, comment added, here's the patch.
> >
> > Thanks, LGTM.  Po Lu, any objections?
> 
> None here.  Please go ahead and install it.

Thanks, installed on the emacs-29 branch, and closing the bug.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> Resent-To: bug-gnu-emacs@gnu.org
> Cc: sbaugh@catern.com, 64423-done@debbugs.gnu.org, sbaugh@janestreet.com
> Date: Sat, 15 Jul 2023 12:01:31 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Po Lu <luangruo@yahoo.com>
> > Cc: sbaugh@catern.com,  sbaugh@janestreet.com,  64423@debbugs.gnu.org
> > Date: Sat, 15 Jul 2023 16:33:13 +0800
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > >> From: sbaugh@catern.com
> > >> Date: Thu, 13 Jul 2023 22:39:10 +0000 (UTC)
> > >> Cc: luangruo@yahoo.com, sbaugh@janestreet.com, 64423@debbugs.gnu.org
> > >> 
> > >> > Fine by me, but please add a comment there explaining why we do
> > >> > that
> > >> > on X.
> > >> >
> > >> > Thanks.
> > >> 
> > >> OK, comment added, here's the patch.
> > >
> > > Thanks, LGTM.  Po Lu, any objections?
> > 
> > None here.  Please go ahead and install it.
> 
> Thanks, installed on the emacs-29 branch, and closing the bug.

This causes the following warning to be emitted (on master):

  In kill-new:
  simple.el:5660:38: Warning: ‘ignore-error’ condition argument should not be quoted: 'quit

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[sbaugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> Resent-To: bug-gnu-emacs@gnu.org
>> Cc: sbaugh@catern.com, 64423-done@debbugs.gnu.org, sbaugh@janestreet.com
>> Date: Sat, 15 Jul 2023 12:01:31 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> 
>> > From: Po Lu <luangruo@yahoo.com>
>> > Cc: sbaugh@catern.com,  sbaugh@janestreet.com,  64423@debbugs.gnu.org
>> > Date: Sat, 15 Jul 2023 16:33:13 +0800
>> > 
>> > Eli Zaretskii <eliz@gnu.org> writes:
>> > 
>> > >> From: sbaugh@catern.com
>> > >> Date: Thu, 13 Jul 2023 22:39:10 +0000 (UTC)
>> > >> Cc: luangruo@yahoo.com, sbaugh@janestreet.com, 64423@debbugs.gnu.org
>> > >> 
>> > >> > Fine by me, but please add a comment there explaining why we do
>> > >> > that
>> > >> > on X.
>> > >> >
>> > >> > Thanks.
>> > >> 
>> > >> OK, comment added, here's the patch.
>> > >
>> > > Thanks, LGTM.  Po Lu, any objections?
>> > 
>> > None here.  Please go ahead and install it.
>> 
>> Thanks, installed on the emacs-29 branch, and closing the bug.
>
> This causes the following warning to be emitted (on master):
>
>   In kill-new:
>   simple.el:5660:38: Warning: ‘ignore-error’ condition argument should
> not be quoted: 'quit

Sorry, here's the fix:

diff --git a/lisp/simple.el b/lisp/simple.el
index 54e71e1b040..6dc08ff0eb0 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -5657,7 +5657,7 @@ kill-new
                   ;; interrupt this. If they interrupt it, we want to continue
                   ;; so we become selection owner, so this doesn't stay slow.
                   (if (eq (window-system) 'x)
-                      (ignore-error 'quit (funcall interprogram-paste-function))
+                      (ignore-error quit (funcall interprogram-paste-function))
                     (funcall interprogram-paste-function)))))
         (when interprogram-paste
           (setq interprogram-paste

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Sat, 15 Jul 2023 17:38:01 +0000 (UTC)
> Cc: 64423@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > This causes the following warning to be emitted (on master):
> >
> >   In kill-new:
> >   simple.el:5660:38: Warning: ‘ignore-error’ condition argument should
> > not be quoted: 'quit
> 
> Sorry, here's the fix:

Thanks, installed.  But does this mean the original change was not
really tested?

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Spencer Baugh]

[-- Attachment #1: Type: text/html, Size: 1134 bytes --]

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Mattias Engdegård]

> I tested and it works fine for me both with and without this edit. I guess this warning is just a style thing.

An ignore-error argument of 'quit is reader sugar for (quote quit) which will ignore both conditions `quote` and `quit` which probably wasn't intended. It works but is a bit wasteful.

bug#64423: 29.0.92; save-interprogram-paste-before-kill doesn't prevent streaming large selections

[Spencer Baugh]

FYI, just for completeness and to help future users, I managed to track
down the original source of this "large selection" for me, which was
causing gui-get-selection to hang.  It's this bug in xrdp when an image
is copied:

https://github.com/neutrinolabs/xrdp/issues/2763

This actually is just a hang with no data, not a large selection.  So in
the end, preventing the streaming of large selections wouldn't have
helped.

The patch that we did end up applying, to ignore quit in
gui-get-selection in kill-new, works great at mitigating this xrdp bug
though.