-
-
Notifications
You must be signed in to change notification settings - Fork 50
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
Clear global clipboard after specified seconds #158
base: main
Are you sure you want to change the base?
Clear global clipboard after specified seconds #158
Conversation
Last second thoughts: If we store
|
Interesting idea. The copy notification could also say something like "... and will be cleared in XX seconds". |
In this case, I think it is better to have dedicated key for it in the JSON (like) metadata. Easy to implement. I will do it. |
0b77a64
to
f215e31
Compare
Further notes: |
f215e31
to
9591979
Compare
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024111418-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024091704-4.3&flavor=update
Failed tests5 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/112766#dependencies 201 fixed
Unstable tests
|
perror("Can not get clipboard data file status"); | ||
exit(1); | ||
} | ||
switch (g->clipboard_timeout_pid = fork()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but if you clear the pid variable on SIGCHLD, depending on the execution order, it may override the new PID instead of the old one, so be careful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I am losing this and I need to study a lot. The way I did it at the beginning was to assure that the clearing child process(es) assure that the clipboard content belongs to their original intended clearing event. Otherwise simply exit. The disadvantage would have been possibility for multiple useless waiting child processes.
Trying to keep track of a clearing child process and terminating it (and nothing else) is something I have difficulty with doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using a subprocess, I recommend using a timer integrated with the event loop. This is much cleaner and avoids all of the problems you are having with subprocesses. Using fork()
for concurrency is something Qubes OS does a lot, but it’s generally considered bad practice nowadays. Newer programs usually an event loop.
The two ways to deal with timeouts are to either use a timeout for poll()
(updated each call) or a Linux timerfd.
9591979
to
fa0af81
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid using fork()
. Managing child processes creates a lot of complexity and isn’t needed here.
perror("Can not get clipboard data file status"); | ||
exit(1); | ||
} | ||
switch (g->clipboard_timeout_pid = fork()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using a subprocess, I recommend using a timer integrated with the event loop. This is much cleaner and avoids all of the problems you are having with subprocesses. Using fork()
for concurrency is something Qubes OS does a lot, but it’s generally considered bad practice nowadays. Newer programs usually an event loop.
The two ways to deal with timeouts are to either use a timeout for poll()
(updated each call) or a Linux timerfd.
Actually, there is one benefit of using |
But also, the child process could be used just for the case of early qube shutdown, and otherwise use the event loop. |
Alternatively the process could keep running even if the qube shut down, or just wipe the clipboard on qube shutdown. @marmarta: what do you think of wiping the clipboard when the qube that pasted into it is shut down? |
I already asked, see my comment above.
That may be messy to do. You'll need to do most cleanup (destroy windows, unlink pidfile etc) but not the actual exit. But I don't have much better idea (keeping a child running is quite similar). |
I have been thinking about this. The way we are trying to do it is wrong. We do not need multiple clipboard clearing processes or even one per GUI daemon instance. We only need one per entire GUIVM since there is only one instance of global clipboard. On a new copy action, the GUI daemon instance handling the copy action should check if the mentioned clearing process exists and terminate it gracefully before overwriting the global clipboard. Then launch a new one if needed. This program is not necessarily the GUI daemon itself. It does not even require connection to X Window. |
fa0af81
to
e679a35
Compare
fixes: QubesOS/qubes-issues#6641