mdlincoln / clipr

R functions for reading and writing from the system clipboard
http://matthewlincoln.net/clipr/
152 stars 24 forks source link

Process hygiene on linux #36

Closed jennybc closed 5 years ago

jennybc commented 6 years ago

I'm just parking this here for the future, so feel free to close if it bothers you to leave it open. But, as you know, on linux, the use of clipr launches but does not close a process (via xsel and/or xclip). Which has caused trouble on CRAN with packages that actually exercise clipr code via example, test, or vignette. Everyone's current solution seems to be shutting down or removing all such code from CRAN. That is expedient and necessary, but doesn't really seem like a solution that leads to software quality.

Food for future thought, via @kevinushey

IIUC the issue with xclip is that it works by forking itself into a background process, and then hangs around to service clipboard requests from the user. Since it forks itself to the background it never gets closed even if the parent process is closed.

In X11 (if I understand correctly) there isn't really a concept of 'writing to the clipboard', it's more that 'there are processes hanging around that service read / write requests to the clipboard'. So once xclip dies it can no longer provide a clipboard selection; hence why it wants to stick around.

I think the fix here for clipr would be to explicitly kill the xclip / xsel processes when the R session is closed / when its namespace is unloaded or something like that. There is probably a clean way of managing the process lifetime using processx or similar.

jennybc commented 6 years ago

If you want the child process to clean itself up, you can use PR_SET_PDEATHSIG (on Linux...)

Another tidbit from @jmcphers. This is quite interesting because I suspect clipr only has a "process hygiene" problem on Linux (vs. macOS or Windows).

mdlincoln commented 6 years ago

This all makes 100% sense when on CRAN.

But in interactive use, wouldn't this create unexpected behavior if one didn't realize that clipr would wipe your clipboard on X11 and X11 only on closing the R session?

jennybc commented 6 years ago

Confirming: when you say "wipe", I assume you mean "kill" (versus just remove the clipboard contents) (?).

I think processx (or actually the ps package) will provide a way for clipr to mark an xsel or xclip process that it launches (vs. a pre-existing one). And then presumably it would be safe / not shocking for clipr to kill such a process that it basically "owns".

Again, I wouldn't do anything about this now but would reconsider when there are better facilities for process management from R.

jennybc commented 6 years ago

cc @gaborcsardi

gaborcsardi commented 6 years ago

So, dev versions of processx and ps have functionality to: