IQA-463: Use APR file I/O for downloaded viewer installer .exe.

On Windows, calling CreateProcess(bInheritHandles=FALSE) is the wrong idea. In
that case, CreateProcess() passes NO handles -- even the files you've
explicitly designated as the child's stdin, stdout, stderr in the STARTUPINFO
struct! Remove LLProcess code to tweak bInheritHandles; we should also remove
the corresponding (useless) APR extension.
Instead, given that the Windows file-locking problem we've observed is
specific to the viewer installer .exe file downloaded by the background
updater logic, use APR file I/O for that specific file. Empirically, both
llofstream and std::ofstream seem to make the open file handle inheritable;
but apr_file_open() documentation says: "By default, the returned file
descriptor will not be inherited by child processes created by
apr_proc_create()." And indeed, it does appear to sidestep the locking problem.

1 files changed, 6 insertions, 20 deletions

diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp
index e96b328365..760be6da9b 100644
--- a/indra/llcommon/llprocess.cpp
+++ b/indra/llcommon/llprocess.cpp
@@ -282,9 +282,13 @@ public:
 
 	virtual std::string read(size_type len)
 	{
-		// Read specified number of bytes into a buffer. Make a buffer big
-		// enough.
+		// Read specified number of bytes into a buffer.
 		size_type readlen((std::min)(size(), len));
+		// Formally, &buffer[0] is invalid for a vector of size() 0. Exit
+		// early in that situation.
+		if (! readlen)
+			return "";
+		// Make a buffer big enough.
 		std::vector<char> buffer(readlen);
 		mStream.read(&buffer[0], readlen);
 		// Since we've already clamped 'readlen', we can think of no reason
@@ -535,24 +539,6 @@ LLProcess::LLProcess(const LLSDOrParams& params):
 	apr_procattr_t *procattr = NULL;
 	chkapr(apr_procattr_create(&procattr, gAPRPoolp));
 
-#if ! defined(APR_HAS_PROCATTR_INHERIT_SET)
-	// Our special preprocessor symbol isn't even defined -- wrong APR
-	LL_WARNS("LLProcess") << "This version of APR lacks Linden apr_procattr_inherit_set() extension" << LL_ENDL;
-#elif ! APR_HAS_PROCATTR_INHERIT_SET
-	// Symbol is defined, but to 0: expect apr_procattr_inherit_set() to
-	// return APR_ENOTIMPL.
-	LL_DEBUGS("LLProcess") << "apr_procattr_inherit_set() not supported on this platform" << LL_ENDL;
-#else  // APR_HAS_PROCATTR_INHERIT_SET nonzero
-	// As of 2012-04-17, the original Windows implementation of
-	// apr_proc_create() unconditionally passes TRUE for bInheritHandles. That
-	// seems to assume that all files are opened by APR so you can
-	// individually control whether each is inherited by a child process. But
-	// we've been burned by having surprising open file handles inherited by
-	// our child processes. Turn that OFF for us!
-	LL_DEBUGS("LLProcess") << "Setting apr_procattr_inherit_set(0)" << LL_ENDL;
-	ll_apr_warn_status(apr_procattr_inherit_set(procattr, 0));
-#endif
-
 	// For which of stdin, stdout, stderr should we create a pipe to the
 	// child? In the viewer, there are only a couple viable
 	// apr_procattr_io_set() alternatives: inherit the viewer's own stdxxx