From 21c70d377620339c644cf13afebb15395d488542 Mon Sep 17 00:00:00 2001
From: Andrew Ruder <andy@aeruder.net>
Date: Thu, 24 May 2018 20:36:49 -0700
Subject: [PATCH] Use IPC::Run3::run3 rather than IPC::Open3::open3

With IPC::Open3::open3 we can create deadlocks in two different situations:

1.) Git command writes more than a pipe buffer of data to
stdout/stderr before reading stdin.

2.) Git command writes more than a pipe buffer of data to stderr
before stdout.

Solving this with Open3 involves dealing with select and a bunch of other
messiness - let's just use IPC::Run3 which solves this problem already.

Fixes issue #100
---
 Makefile.PL          |  4 ++--
 lib/Git/Wrapper.pm   | 36 +++++++++++-------------------------
 t/deadlock.t         | 26 ++++++++++++++++++++++++++
 t/deadlock_helper.sh | 39 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 78 insertions(+), 27 deletions(-)
 create mode 100644 t/deadlock.t
 create mode 100755 t/deadlock_helper.sh

diff --git a/Makefile.PL b/Makefile.PL
index 3609ac8..69803c2 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -31,7 +31,7 @@ my %WriteMakefileArgs = (
     "File::Temp" => 0,
     "File::chdir" => 0,
     "IPC::Cmd" => 0,
-    "IPC::Open3" => 0,
+    "IPC::Run3" => 0,
     "Scalar::Util" => 0,
     "Sort::Versions" => 0,
     "Symbol" => 0,
@@ -65,7 +65,7 @@ my %FallbackPrereqs = (
   "File::chdir" => 0,
   "IO::File" => 0,
   "IPC::Cmd" => 0,
-  "IPC::Open3" => 0,
+  "IPC::Run3" => 0,
   "POSIX" => 0,
   "Scalar::Util" => 0,
   "Sort::Versions" => 0,
diff --git a/lib/Git/Wrapper.pm b/lib/Git/Wrapper.pm
index 8945d40..3b2601a 100644
--- a/lib/Git/Wrapper.pm
+++ b/lib/Git/Wrapper.pm
@@ -13,7 +13,7 @@ delete $ENV{GIT_PAGER_IN_USE};
 
 use File::chdir;
 use File::Temp;
-use IPC::Open3      qw();
+use IPC::Run3 ();
 use Scalar::Util    qw(blessed);
 use Sort::Versions;
 use Symbol;
@@ -87,41 +87,27 @@ sub RUN {
   my( $parts , $stdin ) = _parse_args( $cmd , @_ );
 
   my @cmd = ( $self->git , @$parts );
-
   my( @out , @err );
 
   {
     local $CWD = $self->dir unless $cmd eq 'clone';
 
-    my ($wtr, $rdr, $err);
-
-    local *TEMP;
-    if ($^O eq 'MSWin32' && defined $stdin) {
-      my $file = File::Temp->new;
-      $file->autoflush(1);
-      $file->print($stdin);
-      $file->seek(0,0);
-      open TEMP, '<&=', $file;
-      $wtr = '<&TEMP';
-      undef $stdin;
-    }
-
-    $err = Symbol::gensym;
-
     print STDERR join(' ',@cmd),"\n" if $DEBUG;
 
+    my ($stdout, $stderr);
     # Prevent commands from running interactively
     local $ENV{GIT_EDITOR} = ' ';
+    IPC::Run3::run3(\@cmd, \$stdin, \$stdout, \$stderr);
 
-    my $pid = IPC::Open3::open3($wtr, $rdr, $err, @cmd);
-    print $wtr $stdin
-      if defined $stdin;
-
-    close $wtr;
-    chomp(@out = <$rdr>);
-    chomp(@err = <$err>);
+    @out = map {
+      chomp;
+      $_;
+    } split(/\n/, $stdout);
 
-    waitpid $pid, 0;
+    @err = map {
+      chomp;
+      $_;
+    } split(/\n/, $stderr);
   };
 
   print "status: $?\n" if $DEBUG;
diff --git a/t/deadlock.t b/t/deadlock.t
new file mode 100644
index 0000000..9654f37
--- /dev/null
+++ b/t/deadlock.t
@@ -0,0 +1,26 @@
+use strict;
+use warnings;
+use Test::More;
+use Git::Wrapper;
+
+my $git = Git::Wrapper->new(".", git_binary => "./t/deadlock_helper.sh");
+
+sub _timeout (&) {
+    my ($code) = @_;
+
+    my $timeout = 0;
+    eval {
+        local $SIG{ALRM} = sub { $timeout = 1; die "TIMEOUT\n" };
+        # 5 seconds should be more than enough time to fail properly
+        alarm 5;
+        $code->();
+        alarm 0;
+    };
+
+    return $timeout;
+}
+
+my $timeout = _timeout { $git->RUN("test", -STDIN => "test1\ntest2\n") };
+is $timeout, 0, "didn't deadlock";
+
+done_testing();
diff --git a/t/deadlock_helper.sh b/t/deadlock_helper.sh
new file mode 100755
index 0000000..7cdd72a
--- /dev/null
+++ b/t/deadlock_helper.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+
+# This script is to help assist the deadlock test.
+# Write a large amount of text to stdout/stderr and read from stdin and
+# then repeat.
+
+line="All work and no play makes Jack a dull boy"
+
+i=1
+while test $i -le 1024; do
+  echo 1-STDOUT$i $line
+  i=$((i+1))
+done
+
+i=1
+while test $i -le 1024; do
+  echo 1-STDERR$i $line
+  i=$((i+1))
+done >&2
+
+echo -n "Reading input: "
+read empty
+
+i=1
+while test $i -le 1024; do
+  echo 2-STDOUT$i $line
+  i=$((i+1))
+done
+
+i=1
+while test $i -le 1024; do
+  echo 2-STDERR$i $line
+  i=$((i+1))
+done >&2
+
+echo -n "Reading input: "
+read empty
+
+exit 0