Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the problem for large buffers #57

Merged
merged 1 commit into from
Jul 16, 2019
Merged

Conversation

hwangcc23
Copy link
Contributor

@hwangcc23 hwangcc23 commented May 13, 2019

Fix the issue #43(#43).

Fix the issue astrand#43(astrand#43).

The implementation of the INCR mechanism in xcin() as-is could only service ONE requestor at one time.

For example, when running the command dd 'if=/dev/zero' 'bs=2000' 'count=1000' | tr '\000' 'a' | ./xclip -i,
'xclip -i' was in the XCLIB_XCIN_INCR context of xcin() for the requestor A (whose id is 0x200001 on my environment).
	======== X events from xtrace ========
	Waiting for selection requests, Control-C to quit
	  Waiting for selection request number 1
	000:<:0008: 40: Request(1): CreateWindow depth=0x00 window=0x02c00001 parent=0x0000016e x=0 y=0 width=1 height=1 border-width=0 class=CopyFromParent(0x0000) visual=CopyFromParent(0x00000000) value-list={background-pixel=0x00000000 border-pixel=0x00000000}
	000:<:0009: 16: Request(2): ChangeWindowAttributes window=0x02c00001 value-list={event-mask=PropertyChange}
	000:<:000a: 16: Request(22): SetSelectionOwner owner=0x02c00001 selection=0x1("PRIMARY") time=CurrentTime(0x00000000)
	000:>:000a: Event SelectionRequest(30) time=CurrentTime(0x00000000) owner=0x02c00001 requestor=0x00200001 selection=0x1("PRIMARY") target=0x157("UTF8_STRING") property=0x15c(unrecognized atom)
	======================================

Later, requests from another requestor B 'xclip -o' (whose id is 0x03200001 on my environment) were all ignored by xclip -i.
	======== X events from xtrace ========
	000:>:000f: Event SelectionRequest(30) time=CurrentTime(0x00000000) owner=0x02c00001 requestor=0x03200001 selection=0x1("PRIMARY") target=0x157("UTF8_STRING") property=0x2c3(unrecognized atom)
	======================================

This caused the requestor B 'xclip -o' hung up.

Fix the problem by allocating a new context for every requestor.
@hwangcc23 hwangcc23 changed the title [WIP]Fix the problem for large buffers Fix the problem for large buffers May 16, 2019
@hwangcc23
Copy link
Contributor Author

Ready to go.
@astrand please help review. Thanks.

@hwangcc23
Copy link
Contributor Author

This PR should also fix the issue #34.

@astrand
Copy link
Owner

astrand commented May 22, 2019

Thanks a lot for working on this! Unfortunately, since the fix was non-trivial and I'm very short on time now, it will be a little bit difficult to merge this right away. Are there any other people interested in trying out the fix and/or reviewing the code?

@RogueScholar
Copy link

@astrand I built from hwangcc23:fix-43 just over two weeks ago and haven't seen any issues or regressions in the interim, and it certainly does handle larger selections more efficiently since doing so. I'm not conversant enough in C to speak to the robustness of the code changes themselves, but all's well that ends well in my book. 😏

@hwangcc23 Nicely done. This wasn't an issue that concerned me very often, but it's nice to know I can go hog wild with copy/pastes from the terminal going forward if the situation calls for it. 🥇

@hwangcc23
Copy link
Contributor Author

RogueScholar had tried the patch and not seen any issue. @astrand Is there any chance in merging this patch? Thanks.

@astrand astrand merged commit 6623ccd into astrand:master Jul 16, 2019
@astrand
Copy link
Owner

astrand commented Jul 16, 2019

Merged now, thanks for your contribution.

@mckellyln
Copy link

@astrand hi.
I seem to get xclip running forever after this change.
There are a few requestors and finished is 1 for one of them but then it loops forever due to the others.

@mckellyln
Copy link

I think if sloop > 0 then we need to always break out of while() when finished is true ?

@hwangcc23 hwangcc23 deleted the fix-43 branch July 23, 2019 14:08
hwangcc23 added a commit to hwangcc23/xclip that referenced this pull request Jul 24, 2019
The -loops option is broken after merging the pull-request astrand#57 (astrand#57).

Here is a sample transcript to reproduce the problem.
hwangcc23-BU401LG:~$ dd 'if=/dev/zero' 'bs=2000' 'count=1000' | tr '\000' 'a' | xclip -i -quiet -l 1

Expect: xclip should exist right after any requestor (ex: `xclip -o`) retrieved data.
What I saw: xclip didn't exist.

Break out the while(1) loop once there is one SelectionRequest completed such that dloop can be decreased by one and compared to sleep for fixing the problem.
astrand pushed a commit that referenced this pull request Aug 2, 2019
The -loops option is broken after merging the pull-request #57 (#57).

Here is a sample transcript to reproduce the problem.
hwangcc23-BU401LG:~$ dd 'if=/dev/zero' 'bs=2000' 'count=1000' | tr '\000' 'a' | xclip -i -quiet -l 1

Expect: xclip should exist right after any requestor (ex: `xclip -o`) retrieved data.
What I saw: xclip didn't exist.

Break out the while(1) loop once there is one SelectionRequest completed such that dloop can be decreased by one and compared to sleep for fixing the problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants