SVN: pipe handling reordering, read STDOUT first

Due to limitations on the buffering of pipes and therefore possible
resulting deadlocks, the order in which the pipes are read is crucial.

This reordering of the pipes is just a partial workaround. Processing
STDOUT before STDERR will NOT(!) prevent a deadlock if the output to
STDERR exceeds the pipe's max buffered size.

Regardless, as discussed in #261, it is better to handle an unlikely
error scenario, than to frequently get stuck when processing good
commits.

Signed-off-by: Damien Regad <[email protected]>

- Original commit message and comment block reworded
- Updated changelog
- Bump SourceSVN plugin version

Fixes #333

SourceSVN/SourceSVN.php

@@ -9,7 +9,7 @@
 
 class SourceSVNPlugin extends MantisSourcePlugin {
 
-	const PLUGIN_VERSION = '2.2.0';
+	const PLUGIN_VERSION = '2.3.0';
 	const FRAMEWORK_VERSION_REQUIRED = '2.0.0';
 
 	/**
@@ -338,12 +338,23 @@ private function svn_run( $p_cmd, $p_repo = null )
 			plugin_error( self::ERROR_SVN_RUN );
 		}
 
-		# Get output of the process & clean up
-		$t_stderr = stream_get_contents( $t_pipes[2] );
-		fclose( $t_pipes[2] );
+		# Get the output of the process & clean up
+		# Due to limitations on the buffering of pipes (at least on Windows) and
+		# possible resulting deadlocks, the order the pipes are read is crucial.
+		# Processing STDOUT before STDERR will NOT(!) prevent a deadlock if the
+		# output to STDERR exceeds the pipe's max buffered size.
+		# As discussed in #261, it is better to handle an unlikely error
+		# scenario, than to frequently get stuck when processing good commits.
+
+		#STDOUT
 		$t_svn_out = stream_get_contents( $t_pipes[1] );
 		fclose( $t_pipes[1] );
+		#STDERR
+		$t_stderr = stream_get_contents( $t_pipes[2] );
+		fclose( $t_pipes[2] );
+		#STDIN
 		fclose( $t_pipes[0] );
+
 		proc_close( $t_svn_proc );
 
 		# Error handling

docs/CHANGELOG.md

@@ -21,6 +21,8 @@ specification.
 
 - GitHub: authentication using query parameters is deprecated 
   [#335](https://github.com/mantisbt-plugins/source-integration/issues/335)
+- SVN: Workaround to avoid data import failures due to timeout reading proc_open() buffers
+  [#333](https://github.com/mantisbt-plugins/source-integration/issues/333)
 
 
 ## [2.3.0] - 2019-09-06