- Do not keep the environment alive past the process spawn.

runtime/bin/process_patch.dart

Index: runtime/bin/process_patch.dart
===================================================================
--- runtime/bin/process_patch.dart	(revision 34890)
+++ runtime/bin/process_patch.dart	(working copy)
@@ -160,7 +160,6 @@
                Map<String, String> environment,
                bool includeParentEnvironment,
                bool runInShell) {
-    runInShell = identical(runInShell, true);
     if (runInShell) {
       arguments = _getShellArguments(path, arguments);
       path = _getShellCommand();
@@ -193,22 +192,22 @@
     }
 
     _environment = [];
-    if (environment == null) {
-      environment = {};
-    }
-    if (environment is !Map) {
-      throw new ArgumentError("Environment is not a map: $environment");
-    }
-    if (identical(true, includeParentEnvironment)) {
-      environment = new Map.from(Platform.environment)..addAll(environment);
-    }
-    environment.forEach((key, value) {
+    var environmentEntryHandler = (key, value) {

[Anders Johnsen, 2014/04/10 07:06:44]

void environmentEntryHandler(key, value) {
 ...
}

?

[Ivan Posva, 2014/04/10 07:27:38]

They are equivalent.

[Anders Johnsen, 2014/04/10 08:08:12]

Yes, but very odd style (not what we usually use in dart:io).

       if (key is !String || value is !String) {
         throw new ArgumentError(
-            "Environment key or value is not a string: ($key, $value)");
+        "Environment key or value is not a string: ($key, $value)");

[Anders Johnsen, 2014/04/10 07:06:44]

indentation

       }
       _environment.add('$key=$value');
-    });
+    };
+    if (environment != null) {
+      if (environment is !Map) {
+        throw new ArgumentError("Environment is not a map: $environment");
+      }
+      environment.forEach(environmentEntryHandler);
+    }
+    if (includeParentEnvironment) {
+      Platform.environment.forEach(environmentEntryHandler);

[Anders Johnsen, 2014/04/10 07:06:44]

This is wrong. Now parent environment will override child.

[Ivan Posva, 2014/04/10 07:27:38]

I see it now. It was not obvious in the original code. I'll fix this tomorrow.

[Anders Johnsen, 2014/04/10 08:08:12]

I'll start cooking on a test.

+    }
 
     // stdin going to process.
     _stdin = new _StdSink(new _Socket._writePipe());
@@ -321,6 +320,7 @@
                                   _stderr._stream._nativeSocket,
                                   _exitHandler._nativeSocket,
                                   status);
+      _environment = null;  // The environment will not be needed going forward.
       if (!success) {
         completer.completeError(
             new ProcessException(_path,
@@ -376,6 +376,7 @@
                                 _stderr._stream._nativeSocket,
                                 _exitHandler._nativeSocket,
                                 status);
+    _environment = null;  // The environment will not be needed going forward.
     if (!success) {
       throw new ProcessException(_path,
                                  _arguments,