Compatibility with GNU Parallel #1084

Closed
ogoid opened this Issue Nov 1, 2013 · 10 comments

Comments

Projects
None yet
4 participants
@ogoid

ogoid commented Nov 1, 2013

I tried parallelizing some programs with GNU Parallel (http://www.gnu.org/software/parallel/), but fish produces some warnings/errors. For instance:

~> seq 1 6 | parallel echo
3
4
2
1
5
6
fish: Could not connect to universal variable server, already tried manual restart (or no command supplied). You will not be able to share variable values between fish sessions. Is fish properly installed?
~> 

Or:

~> seq 1 6 | parallel --pipe echo
fish: Expected a command name, got token of type 'Run job in background'. Did you mean 'COMMAND; and COMMAND'? See the help section for the 'and' builtin command by typing 'help and'.
Standard input:              test ! -s "/var/folders/w9/9jx0r60d6c1g5rrbkps8rrr00000gn/T/Y6K17QPi1f.chr" && rm -f "/var/folders/w9/9jx0r60d6c1g5rrbkps8rrr00000gn/T/Y6K17QPi1f.chr" && exec true;
                                                                                                          ^
~> 

I'm running OSX 10.9 with fish installed by Homebrew and set as my default shell.

@zanchey

This comment has been minimized.

Show comment
Hide comment
@zanchey

zanchey Nov 5, 2013

Member

For the second problem, Parallel tries to execute a POSIX script in the current $SHELL, but if fish is your login shell then that may not work. You could try setting your shell to bash temporarily:

function parallel;
  set -lx SHELL bash
  command parallel $argv
end

The first problem is proving harder to replicate. It occurs less than 1% of the time (try seq 1 1000 | parallel --gnu -n 1 echo) on one of my machines running fish 2.1.0 and parallel 20120422, and never on my machine running fish 2.1.0-31-g3c65cd4 and parallel 20121122. There is probably a race condition in connecting to fishd which parallel is happening to tickle.

Member

zanchey commented Nov 5, 2013

For the second problem, Parallel tries to execute a POSIX script in the current $SHELL, but if fish is your login shell then that may not work. You could try setting your shell to bash temporarily:

function parallel;
  set -lx SHELL bash
  command parallel $argv
end

The first problem is proving harder to replicate. It occurs less than 1% of the time (try seq 1 1000 | parallel --gnu -n 1 echo) on one of my machines running fish 2.1.0 and parallel 20120422, and never on my machine running fish 2.1.0-31-g3c65cd4 and parallel 20121122. There is probably a race condition in connecting to fishd which parallel is happening to tickle.

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Nov 5, 2013

Member

If we ever do threaded execution #563 we could provide a totally kick-ass builtin parallel.

Member

ridiculousfish commented Nov 5, 2013

If we ever do threaded execution #563 we could provide a totally kick-ass builtin parallel.

@zanchey

This comment has been minimized.

Show comment
Hide comment
@zanchey

zanchey Nov 11, 2013

Member

If you start a huge stack of fish processes, fishd eventually dies with SIGPIPE. Does the fish side of things get done on a background thread or something? I wonder if fish is starting, finishing its work before getting a reply from fishd and tearing everything down. Would it be safe to set SIGPIPE as ignored in the startup of fishd? It will still get EPIPE but that can be safely ignored.

Member

zanchey commented Nov 11, 2013

If you start a huge stack of fish processes, fishd eventually dies with SIGPIPE. Does the fish side of things get done on a background thread or something? I wonder if fish is starting, finishing its work before getting a reply from fishd and tearing everything down. Would it be safe to set SIGPIPE as ignored in the startup of fishd? It will still get EPIPE but that can be safely ignored.

@zanchey

This comment has been minimized.

Show comment
Hide comment
@zanchey

zanchey Nov 11, 2013

Member

This appears to fix the problem:

diff --git a/fishd.cpp b/fishd.cpp
index 5e2a364..30ded3c 100644
--- a/fishd.cpp
+++ b/fishd.cpp
@@ -667,13 +667,14 @@ static void daemonize()
             setup_fork_guards();

             /*
-              Make fishd ignore the HUP signal.
+              Make fishd ignore the HUP and PIPE signals.
             */
             struct sigaction act;
             sigemptyset(& act.sa_mask);
             act.sa_flags=0;
             act.sa_handler=SIG_IGN;
             sigaction(SIGHUP, &act, 0);
+            sigaction(SIGPIPE, &act, 0);

             /*
               Make fishd save and exit on the TERM signal.

Not sure if it's ok though.

Member

zanchey commented Nov 11, 2013

This appears to fix the problem:

diff --git a/fishd.cpp b/fishd.cpp
index 5e2a364..30ded3c 100644
--- a/fishd.cpp
+++ b/fishd.cpp
@@ -667,13 +667,14 @@ static void daemonize()
             setup_fork_guards();

             /*
-              Make fishd ignore the HUP signal.
+              Make fishd ignore the HUP and PIPE signals.
             */
             struct sigaction act;
             sigemptyset(& act.sa_mask);
             act.sa_flags=0;
             act.sa_handler=SIG_IGN;
             sigaction(SIGHUP, &act, 0);
+            sigaction(SIGPIPE, &act, 0);

             /*
               Make fishd save and exit on the TERM signal.

Not sure if it's ok though.

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Nov 11, 2013

Member

I think this fix is the right thing to do. But before we apply it I want to understand why we're getting SIGPIPE. My understanding is that all communication with fishd happens on the main thread and is synchronous, and furthermore that all data sent by fishd is in response to a request from a fish instance (so fishd never initiates). Based on that I'd expect to never get SIGPIPE unless a fish instance crashes, and if we're crashing I very much want to know :)

zanchey, can you share your technique for starting a huge stack of fish instances?

Member

ridiculousfish commented Nov 11, 2013

I think this fix is the right thing to do. But before we apply it I want to understand why we're getting SIGPIPE. My understanding is that all communication with fishd happens on the main thread and is synchronous, and furthermore that all data sent by fishd is in response to a request from a fish instance (so fishd never initiates). Based on that I'd expect to never get SIGPIPE unless a fish instance crashes, and if we're crashing I very much want to know :)

zanchey, can you share your technique for starting a huge stack of fish instances?

@lledey

This comment has been minimized.

Show comment
Hide comment
@lledey

lledey Nov 11, 2013

Contributor

I'm not sure this zanchey's technique (sorry if it's not), but the following makes things go really wrong :

for i in (seq 10)
    fish -c exit &
end

This produces A LOT of error messages and sometimes even backtraces.

Contributor

lledey commented Nov 11, 2013

I'm not sure this zanchey's technique (sorry if it's not), but the following makes things go really wrong :

for i in (seq 10)
    fish -c exit &
end

This produces A LOT of error messages and sometimes even backtraces.

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Nov 11, 2013

Member

Ooh, nice find. Thanks!

Member

ridiculousfish commented Nov 11, 2013

Ooh, nice find. Thanks!

@zanchey

This comment has been minimized.

Show comment
Hide comment
@zanchey

zanchey Nov 12, 2013

Member

As above - I use seq 1 1000 | parallel --gnu -n 1 echo but @lledey's suggestion seems to work too.

The failure is disappointingly intermittent; on my local machine, I am yet to be able to reproduce it, but it is much more frequent on some remote machines.

Member

zanchey commented Nov 12, 2013

As above - I use seq 1 1000 | parallel --gnu -n 1 echo but @lledey's suggestion seems to work too.

The failure is disappointingly intermittent; on my local machine, I am yet to be able to reproduce it, but it is much more frequent on some remote machines.

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Feb 14, 2014

Member

Now I get it - it's very simple. If fishd begins a write before fish starts reading it, and fish exits, then fishd will get SIGPIPE. Ignoring SIGPIPE is the right thing to do.

Member

ridiculousfish commented Feb 14, 2014

Now I get it - it's very simple. If fishd begins a write before fish starts reading it, and fish exits, then fishd will get SIGPIPE. Ignoring SIGPIPE is the right thing to do.

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Feb 15, 2014

Member

Should be fixed by de8bae3 . Thanks for reporting this.

Member

ridiculousfish commented Feb 15, 2014

Should be fixed by de8bae3 . Thanks for reporting this.

@zanchey zanchey modified the milestones: next-minor, next-major Feb 15, 2014

@zanchey zanchey modified the milestones: 2.1.1, next-minor Sep 24, 2014

@zanchey zanchey self-assigned this Sep 24, 2014

@zanchey zanchey added the bug label Sep 24, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment