Finalizing parsing in a compile job should immediately report errors

When preparing compilation, we can't delay error reporting either.

Also put handles during internalization into a deferred handle scope.

BUG=v8:5215
R=marja@chromium.org,rmcilory@chromium.org

Review-Url: https://codereview.chromium.org/2268983002
Cr-Commit-Position: refs/heads/master@{#38817}

src/compiler-dispatcher/compiler-dispatcher-job.cc

@@ -119,7 +119,7 @@ void CompilerDispatcherJob::Parse() {
   status_ = CompileJobStatus::kParsed;
 }
 
-void CompilerDispatcherJob::FinalizeParsingOnMainThread() {
+bool CompilerDispatcherJob::FinalizeParsingOnMainThread() {
   DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
   DCHECK(status() == CompileJobStatus::kParsed);
 
@@ -130,48 +130,12 @@ void CompilerDispatcherJob::FinalizeParsingOnMainThread() {
 
   if (parse_info_->literal() == nullptr) {
     status_ = CompileJobStatus::kFailed;
-    return;
-  }
-
-  InternalizeParsingResult();
-
+  } else {
     status_ = CompileJobStatus::kReadyToCompile;
-}
-
-void CompilerDispatcherJob::ReportErrorsOnMainThread() {
-  DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
-  DCHECK(status() == CompileJobStatus::kFailed);
-
-  // Internalizing the parsing result will throw the error.
-  InternalizeParsingResult();
-
-  status_ = CompileJobStatus::kDone;
-}
-
-void CompilerDispatcherJob::ResetOnMainThread() {
-  DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
-
-  parser_.reset();
-  unicode_cache_.reset();
-  character_stream_.reset();
-  parse_info_.reset();
-  zone_.reset();
-
-  if (!source_.is_null()) {
-    i::GlobalHandles::Destroy(Handle<Object>::cast(source_).location());
-    source_ = Handle<String>::null();
   }
 
-  status_ = CompileJobStatus::kInitial;
-}
-
-void CompilerDispatcherJob::InternalizeParsingResult() {
-  DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
-  DCHECK(status() == CompileJobStatus::kParsed ||
-         status() == CompileJobStatus::kFailed);
-
-  HandleScope scope(isolate_);
-
+  DeferredHandleScope scope(isolate_);
+  {
     // Create a canonical handle scope before internalizing parsed values if
     // compiling bytecode. This is required for off-thread bytecode generation.
     std::unique_ptr<CanonicalHandleScope> canonical;
@@ -193,6 +157,28 @@ void CompilerDispatcherJob::InternalizeParsingResult() {
     parser_.reset();
     unicode_cache_.reset();
     character_stream_.reset();
+  }
+  handles_from_parsing_.reset(scope.Detach());
+
+  return status_ != CompileJobStatus::kFailed;
+}
+
+void CompilerDispatcherJob::ResetOnMainThread() {
+  DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
+
+  parser_.reset();
+  unicode_cache_.reset();
+  character_stream_.reset();
+  parse_info_.reset();
+  zone_.reset();
+  handles_from_parsing_.reset();
+
+  if (!source_.is_null()) {
+    i::GlobalHandles::Destroy(Handle<Object>::cast(source_).location());
+    source_ = Handle<String>::null();
+  }
+
+  status_ = CompileJobStatus::kInitial;
 }
 
 }  // namespace internal

src/compiler-dispatcher/compiler-dispatcher-job.h

@@ -50,11 +50,9 @@ class CompilerDispatcherJob {
   // Transition from kReadyToParse to kParsed.
   void Parse();
 
-  // Transition from kParsed to kReadyToCompile (or kFailed).
-  void FinalizeParsingOnMainThread();
-
-  // Transition from kFailed to kDone.
-  void ReportErrorsOnMainThread();
+  // Transition from kParsed to kReadyToCompile (or kFailed). Returns false
+  // when transitioning to kFailed. In that case, an exception is pending.
+  bool FinalizeParsingOnMainThread();
 
   // Transition from any state to kInitial and free all resources.
   void ResetOnMainThread();
@@ -62,8 +60,6 @@ class CompilerDispatcherJob {
  private:
   FRIEND_TEST(CompilerDispatcherJobTest, ScopeChain);
 
-  void InternalizeParsingResult();
-
   CompileJobStatus status_ = CompileJobStatus::kInitial;
   Isolate* isolate_;
   Handle<JSFunction> function_;  // Global handle.
@@ -76,6 +72,7 @@ class CompilerDispatcherJob {
   std::unique_ptr<Utf16CharacterStream> character_stream_;
   std::unique_ptr<ParseInfo> parse_info_;
   std::unique_ptr<Parser> parser_;
+  std::unique_ptr<DeferredHandles> handles_from_parsing_;
 
   bool can_parse_on_background_thread_;
 

test/unittests/compiler-dispatcher/compiler-dispatcher-job-unittest.cc

@@ -92,7 +92,7 @@ TEST_F(CompilerDispatcherJobTest, StateTransitions) {
   ASSERT_TRUE(job->status() == CompileJobStatus::kReadyToParse);
   job->Parse();
   ASSERT_TRUE(job->status() == CompileJobStatus::kParsed);
-  job->FinalizeParsingOnMainThread();
+  ASSERT_TRUE(job->FinalizeParsingOnMainThread());
   ASSERT_TRUE(job->status() == CompileJobStatus::kReadyToCompile);
   job->ResetOnMainThread();
   ASSERT_TRUE(job->status() == CompileJobStatus::kInitial);
@@ -105,14 +105,14 @@ TEST_F(CompilerDispatcherJobTest, SyntaxError) {
 
   job->PrepareToParseOnMainThread();
   job->Parse();
-  job->FinalizeParsingOnMainThread();
-
+  ASSERT_FALSE(job->FinalizeParsingOnMainThread());
   ASSERT_TRUE(job->status() == CompileJobStatus::kFailed);
-  ASSERT_FALSE(i_isolate()->has_pending_exception());
-  job->ReportErrorsOnMainThread();
-  ASSERT_TRUE(job->status() == CompileJobStatus::kDone);
   ASSERT_TRUE(i_isolate()->has_pending_exception());
+
   i_isolate()->clear_pending_exception();
+
+  job->ResetOnMainThread();
+  ASSERT_TRUE(job->status() == CompileJobStatus::kInitial);
 }
 
 TEST_F(CompilerDispatcherJobTest, ScopeChain) {
@@ -133,7 +133,7 @@ TEST_F(CompilerDispatcherJobTest, ScopeChain) {
 
   job->PrepareToParseOnMainThread();
   job->Parse();
-  job->FinalizeParsingOnMainThread();
+  ASSERT_TRUE(job->FinalizeParsingOnMainThread());
   ASSERT_TRUE(job->status() == CompileJobStatus::kReadyToCompile);
 
   const AstRawString* var_x =