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.