clipboard-master icon indicating copy to clipboard operation
clipboard-master copied to clipboard

On 4.0.0-beta.4, Linux is spamming event handler with same clipboard value.

Open gnattu opened this issue 1 year ago • 8 comments

On other platforms, we have the assumptions that the event handler callback is called when there is any change in the clipboard. However on Linux, version 4.0.0-beta.4 will trigger the callback every sleep_interval seconds. I've noticed that the load_wait is explicitly changed to load in the new version which leads to this behavior. What is that for? Is load_wait doing anything unexpected?

gnattu avatar Feb 04 '24 17:02 gnattu

OK. Probably I understood what happened here. Is the load_wait blocking the thread so that the thread cannot receive the shutdown signal? Can we use something like futures::select! here?

gnattu avatar Feb 04 '24 17:02 gnattu

Well, it seems like we have more problems for this. See quininer/x11-clipboard#36.

The upstream x11-clipboard does not like our current implementation where we are dropping clipboard.

gnattu avatar Feb 04 '24 18:02 gnattu

@gnattu It was changed to allow ability to shutdown gracefully In theory if there is no change it should timeout here https://github.com/DoumanAsh/clipboard-master/blob/master/src/master/x11.rs#L85

But is that not the case for you? I'm not sure why it would return Ok on timeout

Well, it seems like we have more problems for this. See https://github.com/quininer/x11-clipboard/issues/36. The upstream x11-clipboard does not like our current implementation where we are dropping clipboard.

It is just broken shit then I guess I should have reviewed code, but as I only use this library on windows, I never checked how bad x11-clipbboard 😫

Can we use something like futures::select! here?

No, we do not operate on futures here and x11-clipboard is unlikely to have any sort of Future compatible interface

DoumanAsh avatar Feb 04 '24 23:02 DoumanAsh

@gnattu I made work-around for broken destructor https://github.com/DoumanAsh/clipboard-master/commit/8f867520520ecb80d846e60329d664ed89107204

But timeout behavior needs some serious consideration

DoumanAsh avatar Feb 05 '24 09:02 DoumanAsh

It seems like the load api does require the clipboard content to be changed but will return whatever in the clipboard and the moment of polling as the API calling is just called a "paste" operation. https://github.com/quininer/x11-clipboard/blob/master/examples/paste.rs

In my own fork I'm using a hasher to hash the contents it returned and check if the hash is changed then trigger the callback. I'm not sure if the hashing approach is suitable for all clipboard contents, though. My own fork only targets plain text contents. The rate is close to my sleep_interval is probably just a coincidence that the rate-limit internally is close to my custom value. So we may also want to manually sleep the thread for that time in the handler.

gnattu avatar Feb 06 '24 10:02 gnattu

You generally can use hashing approach with any format as you get content as bytes, but I wanted to omit getting data directly and delegate dealing with it to the user (to avoid dealing with different kinds of formats)

It seems like the load api does require the clipboard content to be changed but will return whatever in the clipboard and the moment of polling as the API calling is just called a "paste" operation.

Ok, seeing example now I understand That is pretty retarded API as no one would expect timeout to be just wait limit to get content... I completely misunderstood the intent behind it

I suppose I need to return load_wait then and probably implement this logic in low level x11 calls

If you have suggestion on how to do it properly let me know I will revert it back to load_wait soon and keep this release in beta until better solution is found

DoumanAsh avatar Feb 06 '24 11:02 DoumanAsh

I released 4.0.0-beta.5 with reverted code to use load_wait Effectively making Shutdown ineffective there

DoumanAsh avatar Feb 06 '24 22:02 DoumanAsh

Simplest alternative probably to use https://docs.rs/x11rb/0.13.0/x11rb/connection/trait.Connection.html#tymethod.poll_for_raw_event_with_sequence

DoumanAsh avatar Feb 07 '24 00:02 DoumanAsh