astrand / xclip

Command line interface to the X11 clipboard
GNU General Public License v2.0
1.03k stars 73 forks source link

Orphaned subprocesses may cause command substitution to hang. #134

Open hongyi-zhao opened 2 years ago

hongyi-zhao commented 2 years ago

On Ubuntu 20.04.3 LTS, I check the self-compiled git master version of xclip with the following bash script:

#! /bin/bash
myclip() {
xclip -selection clipbord -in <<< "hello world"
echo x
return
}
set -x
while true; do
y="$(myclip)"
done

It will be blocked there forever, as shown below. In other words, when running xclip in a function, it cannot return to the main script.

image

Please refer to the related discussions here and here.

Regards, HZ

mviereck commented 2 years ago

The blocking only happens if the output of the function is captured with $(...). It does not happen if the function is called without $(...) in the main loop:

#! /bin/bash
myclip() {
  xclip -selection clipbord -in <<< "hello world" 
  echo x
  return
}
set -x
while true; do
  myclip
done

However, I don't understand why the first example blocks. I assume it is somehow related to xclip forking itself into background. I have tried with nohup xclip ... &, but that does not help.

hongyi-zhao commented 2 years ago

The following script works, see the discussion here for more detailed information:

$ cat myclip.sh
#! /bin/bash
myclip()
{
printf '%s\n' 'hello world' |
xclip -selection clipboard -in > /dev/null
printf '%s\n' x
sleep 1
return
}
set -x
while true; do
y="$(myclip)"
done

$ bash myclip.sh
+ true
++ myclip
++ printf '%s\n' 'hello world'
++ xclip -selection clipboard -in
++ printf '%s\n' x
++ sleep 1
++ return
+ y=x
+ true
++ myclip
++ printf '%s\n' 'hello world'
++ xclip -selection clipboard -in
++ printf '%s\n' x
++ sleep 1
++ return
+ y=x
+ true
++ myclip
++ printf '%s\n' 'hello world'
++ xclip -selection clipboard -in
^C
mviereck commented 2 years ago

I have compared the code. The essential difference is to redirect stdout of xclip to /dev/null:

  xclip -selection clipboard -in <<< "hello world" > /dev/null

That is quite surprising. I wonder if that is a bash or an xclip issue.

hongyi-zhao commented 2 years ago

The following explanation comes from here given by Helmut Waitzmann <Helmut.Waitzmann@web.de>:

The various variants of your example I gave in the messages with the Message‐Ids [83h777t...@helmutwaitzmann.news.arcor.de](https://groups.google.com/) and [83a6czt...@helmutwaitzmann.news.arcor.de](https://groups.google.com/) show: It's not the function that causes the blocking. The script blocks if and only if "xclip" is invoked without redirecting its standard output away from feeding the command substitution "$( … )". Whether that occurs inside of a function or just inline makes no difference.

So, what's going on, there?

When the shell performs a command substition

"$( some commands )",

it first creates and opens a pipe by means of the system call "pipe()". (A pipe is a kernel object which can be written into at one end and read from at the other end. See the manual pages pipe(7) and pipe(2).)

Then the shell forks a process that will execute the subshell environment: the command line inside the "$( )".

The forked subshell environment will write to the created pipe while the outer shell, i. e. the shell that has forked the subshell environment, will continue reading data from the created pipe and collecting it until it reaches an end‐of‐file condition. When it has reached the end‐of‐file condition, it will wait by means of one of the "wait" family of system calls (see the manual page wait(2)) for the subshell environment to terminate.

It puts the collected data it has read from the pipe into the parameter list of the command. In your example that command is the variable assignment:

y="$( … )"

Therefore, the read data will get assigned to the shell variable "y".

The exit status of the variable assignment will be the exit status as obtained from the "wait" system call, i. e. it will be the exit status of the command substitution.

The key to understanding why your example blocks is to understand what an end‐of‐file condition in a pipe is:

When reading from a pipe, there are (at least) three conditions to be distinguished:

(1) A writer has written something to the pipe. Then upon reading, the reader will get that data, which will be removed from the pipe.

(2) The pipe is empty but the writing end is still open for writing, i.e. nothing has been written to it since all data that may have been written to it have been read. Then upon reading, the reader will block and wait for further data to arrive as it does not know whether the writer eventually will write further data.

(3) The writer may have written some (or may not have written any) data to the pipe and then closes the writing end of the pipe. When there is no writer left to write to the writing end of the pipe, then upon reading, the reader will get the data (if there is one) like in (1) and then it will get an end‐of‐file condition.

The end‐of‐file condition tells the reader (in your use case: the outer shell) that there will no further data arrive in the pipe (as no one has it open for writing any more).

In the use case of the command substitution the outer shell will know that it has read all data to be used in the command substitution. It will close its reading end of the pipe and then wait for the subshell environment to terminate.

Now what's going on, when the subshell environment starts the "xclip" command which will fork a subprocess?

Without redirection of its standard output, that is, the blocking use cases, "xclip" will have its standard output file descriptor be the writing end of the pipe. When "xclip" forks its subprocess, the subprocess will of course inherit the writing end of the pipe. => There are two processes which might write to the pipe.

Now, if the "outer" "xclip" process terminates, its writing end of the pipe will implicitly get closed, but the writing end of the pipe hold by the forked "xclip" subprocess will remain open.

=> As written above in (2) and (3) the reader of the pipe won't get an end‐of‐file condition as long as the forked "xclip" subprocess neither closes its standard output (Of course it will not: No program is expected to close its standard output.) nor terminates.

=> Therefore the outer shell will wait for the subshell environment to send an end‐of‐file condition to the pipe until the cows come home.

Now on the other hand, if "xclip" is called with its standard output file descriptor redirected from the writing end of the pipe to for example "/dev/null", then its forked subprocess will inherit the redirected standard output file descriptor.

=> No dangling writing end of the pipe will stay open and the outer shell will finally get its longed‐for end‐of‐file condition when reading from the pipe.

That scenario is not specific to "xclip". Here is an example using "sleep" in the background. Note: "sleep" won't write anything to its standard output, and of course it won't even bother closing it:

( while var="$( sleep -- 2 & date -- '+%Y-%m-%dT%T%z' )" do printf 'command substitution ran at %s\n' "$var" date -- '+command substitution ended at %Y-%m-%dT%T%z' echo sleep -- 1 done )

The output shows that the command substitution lasts for 2 seconds in spite of the "sleep" command being run in the background, because the end‐of‐file condition for the reader is delayed until that two‐seconds‐sleep terminates and implicitly gets it standard output file descriptor, which refers to the writing end of the pipe, closed.

If the standard output of the "sleep" command is redirected from the writing end of the pipe to (for example) "/dev/null" then the command substitution returns almost immediately, as can be seen here:

( while var="$( sleep -- 2 > /dev/null & date -- '+%Y-%m-%dT%T%z' )" do printf 'command substitution ran at %s\n' "$var" date -- '+command substitution ended at %Y-%m-%dT%T%z' echo sleep -- 1 done )

May I add my refrain? Knowing linux or unix greatly helps understanding the shell. Without that knowledge one will have a hard job doing so.

The following is the relevant discussion Helmut Waitzmann sent me via private email on this issue. Because it is related to this issue, I also forward it here for reference:

The same "hanging" effect can be seen when using pipes in the commandline: While in

( sleep -- 2 & date -- '+%Y-%m-%dT%T%z' ) | cat date -- '+%Y-%m-%dT%T%z'

"cat" will wait for two seconds on the end‐of‐file condition, it will almost return immediately in

( sleep -- 2 > /dev/null & date -- '+%Y-%m-%dT%T%z' ) | cat date -- '+%Y-%m-%dT%T%z'

Kind regards

Helmut Waitzmann

gklanderman commented 2 years ago

OP mis-understands how xclip works.

When you run % xclip` -selection clipboard -in <<< "hello world"

you immediately get your prompt back, but xclip remains running in the background (it forked itself) and will keep running so long as it owns the selection. When some other X11 application places data in the clipboard, only then it exits. And at that point, as described by Helmut in detail above, the writing end of the pipe will close, and your $(myclip) will return.

Try this: run your script, and in another window:

% xclip -o -selection clipboard
hello world % xclip -o -selection clipboard
hello world % xclip -o -selection clipboard
hello world

you can read from the clipboard multiple times, and the original xclip from your script keeps serving that (has not exited the $(myclip) yet).

but now do:

% xclip -selection clipbord -in <<< "replace"

and you'll see that the original xclip exits, myclip exits, and y is assigned.

Without knowing exactly what you're trying to achieve, it is hard to advise you, but you may want to look into the '-quiet' and '-loops' options.

'-quiet' will cause xclip not to fork (stays in foreground), and '-loops' will cause it to exit after a given number or readers have read the data it posted to the clipboard.

hongyi-zhao commented 2 years ago

% xclip -selection clipboard -in <<< "hello world"

This will put an additional \n into the clipboard, as described below:

$ man bash
**   Here Strings
       A variant of here documents, the format is:

              [n]<<<word

       The word undergoes tilde expansion, parameter and  variable  expansion,
       command  substitution,  arithmetic expansion, and quote removal.  Path‐
       name expansion and word splitting are not  performed.   The  result  is
       supplied as a single string, with a newline appended, to the command on
       its standard input (or file descriptor n if n is specified).
**
gklanderman commented 2 years ago

FWIW I went thru the open bugs and pull requests yesterday, and saw several that were proposing to have xclip -i close stdout when forking into the background.

So that would "fix" your example in the sense that myclip would no longer block, and instead return immediately, however, then you'll just be forking background xclip processes to replace themselves as fast as your shell can do so, which is almost certainly not what you want. What are you actually trying to accomplish?

mviereck commented 2 years ago

Originally the issue came up in my project x11docker. The overall task to accomplish is to share the clipboard between two X servers.

A few days ago I tried a solution using xclip in a function where it reads from both X servers, compares the clips, sends the latest clip to both, and returns the latest clip. This was the point where I was stuck because the function was blocked by stdout of xclip and I did not understand why.

I don't think the behaviour of xclip should be changed as it is regular unix behaviour. Redirecting stdout already solves the issue as explained well above.

Meanwhile I have a new implementation that does not use a function. FWIW, I have extracted my current implementation as a standalone script with Xephyr as a second X server:

#! /usr/bin/env bash

Hostdisplay=$DISPLAY
Hostxauthority=$XAUTHORITY
Newdisplay=:20
Xclientcookie=""

Xephyr $Newdisplay -ac &
sleep 1
DISPLAY=$Newdisplay xterm &

###############################
# share clipboard between X servers $Hostdisplay and $Newdisplay

X1auth[1]="DISPLAY=$Hostdisplay XAUTHORITY=$Hostxauthority"
X2auth[1]="DISPLAY=$Newdisplay XAUTHORITY=$Xclientcookie"
X1auth[2]="DISPLAY=$Newdisplay XAUTHORITY=$Xclientcookie"
X2auth[2]="DISPLAY=$Hostdisplay XAUTHORITY=$Hostxauthority"
Selection[1]=clipboard
Selection[2]=primary
OtherX[1]=2
OtherX[2]=1

while :; do
  for S in 1 2; do
    for X in 1 2; do
      export ${X1auth[$X]}
      Targets="$(xclip -out -selection ${Selection[$S]} -t TARGETS 2>/dev/null)"

      [ -n "$Targets" ] && {
        # check for image clip
        grep -q "image/png" <<< "$Targets" && Imagetarget="-t image/png" || Imagetarget=""

        # read content of ${Selection[$S]} of X server $X
        Clip[$X$S]="$(xclip -out -selection ${Selection[$S]} $Imagetarget | base64)"

        # check if ${Selection[$S]} of X server $X has changed
        [ -n "${Clip[$X$S]}" ] && [ "${Clipold[$X$S]}" != "${Clip[$X$S]}" ] && {
          Clipold[$X$S]="${Clip[$X$S]}"

          # send only to clipboard of other X server if it has different content. Important to keep highlighted text selection.
          [ "${Clip[${OtherX[$X]}$S]}" != "${Clip[$X$S]}" ] && {
            export ${X2auth[$X]}
            base64 -d <<< "${Clip[$X$S]}" | xclip -in -selection ${Selection[$S]} $Imagetarget
            #echo "SEND FROM $X (${Selection[$S]}): $(base64 -d <<< "${Clip[$X$S]}")" >&2
          }
        }
      }
    done
  done
  # sleep a bit to avoid high cpu usage
  sleep 0.3
done

################ A feature that would help me to improve this: If xclip could optionally behave similar to xclip -in -quiet in a way that it waits for new content before returning (but doing nothing else), I could write a less CPU expensive loop. I think of a new option like xclip -waitforchange that returns once the clipboard content changes. xclip -in -quiet already behaves this way. However, this leads to a special issue if it comes to -selection primary. The loop detects a new content with xclip -out -selection primary and comparing it with previous clipboard content. But if I send this new content with xclip -in -selection primary -quiet to have a 'waiting xclip', the selected text looses its highlighting. (Because the application providing the clip has changed.) This might sound a bit confusing. I'll add a script that helps to reproduce and understand the issue.

gklanderman commented 2 years ago

I don't think the behaviour of xclip should be changed as it is regular unix behaviour. Redirecting stdout already solves the issue as explained well above.

since xclip -i will not write to stdout, I think it actually does make sense to close stdout, and I think that was the conclusion drawn in several bugs, but I did not read them at all carefully.

I think of a new option like xclip -waitforchange that returns once the clipboard content changes.

I implemented something similar (locally) today, so that the -wait option works with -o, but not in exactly the way you want: what I implemented is that if no selection exists when you run xclip -o, it will wait up to the # of millisec specified by -wait, or indefinitely if -wait is negative. It should not be too hard to change that to do what you want (just bypass the "if no current selection" check).

this leads to a special issue if it comes to -selection primary

I think I see what you're getting at, but I can't think of a solution.

btw I just stumbled across this project the other day that sounds similar to what you're trying to do:

https://www.chiark.greenend.org.uk/~sgtatham/utils/xclipglue.html

have you seen it?

mviereck commented 2 years ago

since xclip -i will not write to stdout, I think it actually does make sense to close stdout

At least with option -filter it writes to stdout.

I implemented something similar (locally) today, so that the -wait option works with -o, but not in exactly the way you want: what I implemented is that if no selection exists when you run xclip -o, it will wait up to the # of millisec specified by -wait, or indefinitely if -wait is negative. It should not be too hard to change that to do what you want (just bypass the "if no current selection" check).

That sounds promising! For my purpose -wait could be bound to -out as well; if I don't need the output, I could ignore it.

btw I just stumbled across this project the other day that sounds similar to what you're trying to do:

Thank you for the hint! xclipclue looks promising and I wasn't aware of it. Unfortunately it has bugs. It does not sync the clipboards, instead it breaks the possibility to get context menus with right mouse click.

I think I see what you're getting at, but I can't think of a solution.

Basically I only need a waiting xclip other than with -in -quiet to avoid expensive loops. I cannot use -in for this because it removes the highlighting of text to copy in case of -selection primary.

I've rewritten the code above a bit to split it into 4 loops. Some sort of xclip -out -waitforchange could replace the sleep command. The script would react immediately to clipboard changes without a sleep delay and it would not repeatedly compare the output of xclip -out at high cost to no avail.

#! /usr/bin/env bash

Hostdisplay=$DISPLAY
Hostxauthority=$XAUTHORITY
Newdisplay=:20
Xclientcookie=""

Xephyr $Newdisplay -ac &
sleep 1
DISPLAY=$Newdisplay xterm &

#############################

clipexchange() {
  # Send clipboard content of one X server to another X server and update on changes.
  # Run function twice with switched credentials to sync the clipboards.
  # $1  credentials to access 1. X server
  # $2  credentials to access 2. X server
  # $3  selection to use (primary or clipboard)
  # X credentials look like "DISPLAY=:0 XAUTHORITY=~/.Xauthority"

  local Selection
  Selection="${3:-}"

  while :; do
    Targets="$(env ${1:-} xclip -out -selection $Selection -t TARGETS 2>/dev/null)"

    # wait for the clipboard to have some content at all
    [ -z "$Targets" ] && env ${1:-} xclip -selection $Selection -in -verbose <<< ""

    [ -n "$Targets" ] && {
      # check for image clip
      grep -q "image/png" <<< "$Targets" && Imagetarget="-t image/png" || Imagetarget=""

      # read content of $Selection of X server $1
      Clip="$(env ${1:-} xclip -selection $Selection -out $Imagetarget | base64)"

      # check if $Selection of X server $1 has changed
      [ -n "$Clip" ] && [ "$Clip" != "$Clipold" ] && {
        Clipold="$Clip"

        # send only to clipboard of other X server if it has different content. Important to keep highlighted text selection.
        Clipold2="$(env ${2:-} xclip -selection $Selection -out $Imagetarget | base64)"
        [ "$Clipold2" != "$Clip" ] && {
          #echo "SEND $Selection TO $2"
          base64 -d <<< "$Clip" | env ${2:-} xclip -selection $Selection -in $Imagetarget
        }
      }

      # wait a bit to avoid high CPU usage
      sleep 0.3

      # exit if one X server is not accessible
      env ${1:-} xclip -o >/dev/null || break
      env ${2:-} xclip -o >/dev/null || break
    }
  done
}

X1auth="DISPLAY=$Hostdisplay XAUTHORITY=$Hostxauthority"
X2auth="DISPLAY=$Newdisplay XAUTHORITY=$Xclientcookie"

clipexchange "$X1auth" "$X2auth" clipboard &
clipexchange "$X2auth" "$X1auth" clipboard &
clipexchange "$X1auth" "$X2auth" primary &
clipexchange "$X2auth" "$X1auth" primary &
gklanderman commented 2 years ago

sorry xclipglue didn't work, it really seems like it should do exactly what you want, and should ideally be more robust than a shell script. you might reach out to the author, he last made a change ~1.5 years ago, and his email is at the bottom of this page:

https://www.chiark.greenend.org.uk/~sgtatham/utils/

re: your script, IIUC, you would be able to get rid of the sleep and the xclip -selection $Selection -in -verbose <<< "" with the waitforchange flag?

I have a couple hoops with my employer before I can submit a pull request for the wait feature I implemented. Once I submit that, the additional option you want should be pretty easy to add.

mviereck commented 2 years ago

re: your script, IIUC, you would be able to get rid of the sleep and the xclip -selection $Selection -in -verbose <<< "" with the waitforchange flag?

Yes, exactly.

I have a couple hoops with my employer before I can submit a pull request for the wait feature I implemented. Once I submit that, the additional option you want should be pretty easy to add.

That sounds great! No need to hurry.

gklanderman commented 2 years ago

Sounds good Martin, this week is busy and I'm on vacation next week, so while I might get to it this week, it could take a few weeks.

mviereck commented 2 years ago

@gklanderman Just want to kindly ask if you still have this project in mind?

gklanderman commented 2 years ago

Hi @mviereck sorry I have had no time at all unfortunately.. I will try to make some progress in the next few weeks, ping me again if you don't hear anything..