VM: [Kernel] Fix bootstraping when Kernel isolate is used.

runtime/bin/main.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // 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.
 
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
 
 #include "include/dart_api.h"
 #include "include/dart_tools_api.h"
@@ skipping 109 matching lines @@
 static const int DEFAULT_VM_SERVICE_SERVER_PORT = 8181;
 // VM Service options.
 static const char* vm_service_server_ip = DEFAULT_VM_SERVICE_SERVER_IP;
 // The 0 port is a magic value which results in the first available port
 // being allocated.
 static int vm_service_server_port = -1;
 // True when we are running in development mode and cross origin security
 // checks are disabled.
 static bool vm_service_dev_mode = false;
 
+// Exit code indicating an internal Dart Frontend error.
+static const int kDartFrontendErrorExitCode = 252;
 // Exit code indicating an API error.
 static const int kApiErrorExitCode = 253;
 // Exit code indicating a compilation error.
 static const int kCompilationErrorExitCode = 254;
 // Exit code indicating an unhandled error that is not a compilation error.
 static const int kErrorExitCode = 255;
 // Exit code indicating a vm restart request.  Never returned to the user.
 static const int kRestartRequestExitCode = 1000;
 
 static void ErrorExit(int exit_code, const char* format, ...) {
@@ skipping 664 matching lines @@
     }                                                                          \
     Dart_ExitScope();                                                          \
     Dart_ShutdownIsolate();                                                    \
     return NULL;                                                               \
   }
 
 
 static void SnapshotOnExitHook(int64_t exit_code);
 
 
 // Returns true on success, false on failure.

[kustermann, 2017/01/30 12:14:23]

This comment says it returns true/false, but it seems to be rather returning
some kind of pointer or NULL.

[Vyacheslav Egorov (Google), 2017/01/30 19:34:12]

Done.

 static Dart_Isolate CreateIsolateAndSetupHelper(bool is_main_isolate,
                                                 const char* script_uri,
                                                 const char* main,
                                                 const char* package_root,
                                                 const char* packages_config,
                                                 Dart_IsolateFlags* flags,
                                                 char** error,
                                                 int* exit_code) {
   ASSERT(script_uri != NULL);
-  if (strcmp(script_uri, DART_KERNEL_ISOLATE_NAME) == 0) {
+  const bool is_kernel_isolate =
+      strcmp(script_uri, DART_KERNEL_ISOLATE_NAME) == 0;
+  if (is_kernel_isolate) {
     if (!use_dart_frontend) {
       *error = strdup("Kernel isolate not supported.");
       return NULL;
     } else {
       if (packages_config == NULL) {
         packages_config = commandline_packages_file;
       }
     }
   }
 
 #if defined(DART_PRECOMPILED_RUNTIME)
   // AOT: All isolates start from the app snapshot.
   bool isolate_run_app_snapshot = true;
   const uint8_t* isolate_snapshot_buffer = app_isolate_snapshot_buffer;
 #else
   // JIT: Main isolate starts from the app snapshot, if any. Other use the
   // core libraries snapshot.
   bool isolate_run_app_snapshot = false;
   const uint8_t* isolate_snapshot_buffer = core_isolate_snapshot_buffer;
   if ((app_isolate_snapshot_buffer != NULL) &&
       (is_main_isolate || ((app_script_uri != NULL) &&
                            (strcmp(script_uri, app_script_uri) == 0)))) {
     isolate_run_app_snapshot = true;
     isolate_snapshot_buffer = app_isolate_snapshot_buffer;
   }
 #endif
 
+  const uint8_t* kernel_file = NULL;
+  intptr_t kernel_length = -1;
+  bool is_kernel = false;
+
+  if (use_dart_frontend && !is_kernel_isolate && !is_service_isolate) {
+    Dart_KernelCompilationResult result = Dart_CompileToKernel(script_uri);

[hausner, 2017/01/24 00:50:55]

Calling the front end here only works because it returns a fully linked Kernel
binary, containing the entire core library. Correct?

We want the Dart/Kernel front end to return an incremental Kernel binary
eventually, i.e. a binary that does not contain the core libraries (only
references to the names used in the script). The core libraries will then have
to be loaded from a snapshot again, possibly a new snapshot format which
contains Kernel binary blobs.
 

[Vyacheslav Egorov (Google), 2017/01/30 19:34:12]

Yep, your understanding of the current situation is correct. We are slowly
moving towards application and core split.

+    *error = result.error;  // Copy error message (if any).
+    switch (result.status) {
+      case Dart_KernelCompilationStatus_Ok:
+        is_kernel = true;
+        kernel_file = result.kernel;
+        kernel_length = result.kernel_size;
+        break;
+      case Dart_KernelCompilationStatus_Error:
+        *exit_code = kCompilationErrorExitCode;
+        return NULL;
+      case Dart_KernelCompilationStatus_Crash:
+        *exit_code = kDartFrontendErrorExitCode;
+        return NULL;
+      case Dart_KernelCompilationStatus_Unknown:

[siva, 2017/01/24 02:38:02]

This return code seems to happen only when the kernel isolate itself doesn't
start up. Why not set this up as
*exit_code = kErrorExitCode;


(otherwise exit code ends up being 0 in this case)

[Vyacheslav Egorov (Google), 2017/01/30 19:34:13]

Done.

+        return NULL;
+    }
+  }
+
   // If the script is a Kernel binary, then we will try to bootstrap from the
   // script.
-  const uint8_t* kernel_file = NULL;
-  intptr_t kernel_length = -1;
-  const bool is_kernel =
-      !isolate_run_app_snapshot &&
-      TryReadKernel(script_uri, &kernel_file, &kernel_length);
+  if (!is_kernel && !isolate_run_app_snapshot) {
+    is_kernel = TryReadKernel(script_uri, &kernel_file, &kernel_length);
+  }
 
   void* kernel_program = NULL;
   if (is_kernel) {
     kernel_program = Dart_ReadKernelBinary(kernel_file, kernel_length);
     free(const_cast<uint8_t*>(kernel_file));
   }
 
   IsolateData* isolate_data =
       new IsolateData(script_uri, package_root, packages_config);
   Dart_Isolate isolate =
@@ skipping 63 matching lines @@
   // Setup package root if specified.
   result = DartUtils::SetupPackageRoot(package_root, packages_config);
   CHECK_RESULT(result);
 
   result = Dart_SetEnvironmentCallback(EnvironmentCallback);
   CHECK_RESULT(result);
 
   if (!Dart_IsKernelIsolate(isolate) && use_dart_frontend) {
     // This must be the main script to be loaded. Wait for Kernel isolate
     // to finish initialization.
     Dart_Port port = Dart_ServiceWaitForKernelPort();

[hausner, 2017/01/24 00:50:55]

Haven't we waited for the kernel isolate to be ready when calling
Dart_CompileToKernel() above?

[siva, 2017/01/24 02:38:02]

Maybe move this up to just before where Dart_CompileToKernel is called
and add the kernel_port as a parameter to Dart_CompileToKernel
instead of having this code inside of Dart_CompileToKernel()

[Vyacheslav Egorov (Google), 2017/01/30 19:34:12]

Removed this code.

     if (port == ILLEGAL_PORT) {
       *error = strdup("Error while initializing Kernel isolate");
       return NULL;
     }
+
+    Loader::InitForSnapshot(script_uri);

[hausner, 2017/01/24 00:50:55]

Why is this call needed here? This seems to be app snapshot specific. Maybe the
name of the function should change if it does something that is necessary when
otherwise the loader is not invoked?

[Vyacheslav Egorov (Google), 2017/01/30 19:34:13]

Some parts of core-libraries (e.g. Isolate methods) query loader state to
resolve packages mapping, package relative uris, etc, so we need loader in
initialized state.

I am adding a comment. 

   }
 
   if (isolate_run_app_snapshot) {
     result = DartUtils::SetupIOLibrary(script_uri);
     CHECK_RESULT(result);
     Loader::InitForSnapshot(script_uri);
 #if !defined(DART_PRECOMPILED_RUNTIME)
     if (is_main_isolate) {
       // Find the canonical uri of the app snapshot. We'll use this to decide if
       // other isolates should use the app snapshot or the core snapshot.
       const char* resolved_script_uri = NULL;
       result = Dart_StringToCString(
           DartUtils::ResolveScript(Dart_NewStringFromCString(script_uri)),
           &resolved_script_uri);
       CHECK_RESULT(result);
       ASSERT(app_script_uri == NULL);
       app_script_uri = strdup(resolved_script_uri);
     }
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
   } else {
     // Load the specified application script into the newly created isolate.
     Dart_Handle uri =
         DartUtils::ResolveScript(Dart_NewStringFromCString(script_uri));
     CHECK_RESULT(uri);
     if (!is_kernel) {
       result = Loader::LibraryTagHandler(Dart_kScriptTag, Dart_Null(), uri);
       CHECK_RESULT(result);
     }

[hausner, 2017/01/24 00:50:55]

else {
  // Do the InitForSnapshot() call here?
}

[Vyacheslav Egorov (Google), 2017/01/30 19:34:12]

Done.

 
     Dart_TimelineEvent("LoadScript", Dart_TimelineGetMicros(),
                        Dart_GetMainPortId(), Dart_Timeline_Event_Async_End, 0,
                        NULL, NULL);
 
     result = DartUtils::SetupIOLibrary(script_uri);
     CHECK_RESULT(result);
   }
 
   // Make the isolate runnable so that it is ready to handle messages.
@@ skipping 1064 matching lines @@
   Platform::Exit(Process::GlobalExitCode());
 }
 
 }  // namespace bin
 }  // namespace dart
 
 int main(int argc, char** argv) {
   dart::bin::main(argc, argv);
   UNREACHABLE();
 }