[dartdev] Synchronize environment variable modifications to fix getenv crash

Avoid a race condition between async setenv/unsetenv calls on the VM thread and concurrent getenv/environ reads on the main thread.

In glibc, setenv and getenv are not thread-safe against each other. VmInteropHandler.setEnvironmentVariable previously sent a message to the VM thread to call setenv/unsetenv asynchronously, while the main thread immediately proceeded to access Platform.environment or Platform.localeName (which calls getenv). This could cause a crash (SIGSEGV) in getenv.

This CL makes VmInteropHandler.setEnvironmentVariable synchronous by awaiting a reply from the VM thread before returning.

TEST=pkg/dartdev/test/environment_test.dart

Fixes https://github.com/dart-lang/sdk/issues/63460

Change-Id: Ic8211897ce26ffbdc142fa594cd397189a61f061
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/506800
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Auto-Submit: Ben Konyi <bkonyi@google.com>

pkg/dartdev/lib/dartdev.dart

@@ -48,7 +48,7 @@ Future<void> runDartdev(List<String> args, SendPort? port) async {
   try {
     VmInteropHandler.initialize(port);
     // Set the DART_ROOT environment variable to the SDK path.
-    VmInteropHandler.setEnvironmentVariable('DART_ROOT', sdk.sdkPath);
+    await VmInteropHandler.setEnvironmentVariable('DART_ROOT', sdk.sdkPath);
     // Call the runner to execute the command; see DartdevRunner.
     final runner = DartdevRunner(args, vmArgs: io.Platform.executableArguments);
     exitCode = await runner.run(args);
@@ -220,18 +220,18 @@ class DartdevRunner extends CommandRunner<int> {
     // Since VmInteropHandler.setEnvironmentVariable is non-overwriting by design
     // in C++, we unset the variable first to ensure the explicitly resolved
     // value takes precedence.
-    VmInteropHandler.setEnvironmentVariable(
+    await VmInteropHandler.setEnvironmentVariable(
       DashEnvVar.suppressAnalytics.name,
       null,
     );
 
-    VmInteropHandler.setEnvironmentVariable(
+    await VmInteropHandler.setEnvironmentVariable(
       DashEnvVar.suppressAnalytics.name,
       suppressAnalytics.toString(),
     );
     final envTool = io.Platform.environment[DashEnvVar.tool.name];
     if (envTool == null) {
-      VmInteropHandler.setEnvironmentVariable(
+      await VmInteropHandler.setEnvironmentVariable(
         DashEnvVar.tool.name,
         DashTool.dartTool.label,
       );

pkg/dartdev/lib/src/vm_interop_handler.dart

@@ -2,6 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'dart:async';
 import 'dart:io';
 import 'dart:isolate';
 
@@ -75,11 +76,23 @@ abstract class VmInteropHandler {
   /// Sets the environment variable [name] to [value] for the current process.
   ///
   /// If [value] is null, the environment variable is removed.
-  static void setEnvironmentVariable(String name, String? value) {
+  static Future<void> setEnvironmentVariable(String name, String? value) async {
     final port = _port;
     if (port == null) return;
-    final message = <dynamic>[_kResultSetEnvironmentVariable, name, value];
+    final replyPort = RawReceivePort();
+    final completer = Completer<void>();
+    replyPort.handler = (message) {
+      completer.complete();
+      replyPort.close();
+    };
+    final message = <dynamic>[
+      _kResultSetEnvironmentVariable,
+      replyPort.sendPort,
+      name,
+      value,
+    ];
     port.send(message);
+    await completer.future;
   }
 
   /// This code is identical to the one in process_patch.dart, please ensure

runtime/bin/dartdev.cc

@@ -732,13 +732,22 @@ class DartDev {
   }
 
   static void SetEnvironmentVariableCallback(Dart_CObject* message) {
-    ASSERT(GetArrayItem(message, 1)->type == Dart_CObject_kString);
-    const char* name = GetArrayItem(message, 1)->value.as_string;
+    ASSERT(GetArrayItem(message, 1)->type == Dart_CObject_kSendPort);
+    Dart_Port reply_port = GetArrayItem(message, 1)->value.as_send_port.id;
+
+    ASSERT(GetArrayItem(message, 2)->type == Dart_CObject_kString);
+    const char* name = GetArrayItem(message, 2)->value.as_string;
+
     const char* value = nullptr;
-    if (GetArrayItem(message, 2)->type == Dart_CObject_kString) {
-      value = GetArrayItem(message, 2)->value.as_string;
+    if (GetArrayItem(message, 3)->type == Dart_CObject_kString) {
+      value = GetArrayItem(message, 3)->value.as_string;
     }
+
     Platform::SetEnvironmentVariable(name, value);
+
+    Dart_CObject reply;
+    reply.type = Dart_CObject_kNull;
+    Dart_PostCObject(reply_port, &reply);
   }
 
   // Callback that processes the result from execution of dartdev