Fix issue 142

src/v8threads.cc

 // Copyright 2008 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 251 matching lines @@
   for (ThreadState* state = ThreadState::FirstInUse();
        state != NULL;
        state = state->Next()) {
     char* data = state->data();
     data += HandleScopeImplementer::ArchiveSpacePerThread();
     Top::MarkCompactEpilogue(data);
   }
 }
 
 
+// This is the ContextSwitcher singleton. There is at most a single thread
+// running which delivers preemption events to V8 threads.
+ContextSwitcher* ContextSwitcher::singleton_ = NULL;
+
+
 ContextSwitcher::ContextSwitcher(int every_n_ms)
-  : preemption_semaphore_(OS::CreateSemaphore(0)),
-    keep_going_(true),
+  : keep_going_(true),
     sleep_ms_(every_n_ms) {
 }
 
 
-static v8::internal::ContextSwitcher* switcher;
-
-
+// Set the scheduling interval of V8 threads. This function starts the
+// ContextSwitcher thread if needed.
 void ContextSwitcher::StartPreemption(int every_n_ms) {
   ASSERT(Locker::IsLocked());
-  if (switcher == NULL) {
-    switcher = new ContextSwitcher(every_n_ms);
-    switcher->Start();
+  if (singleton_ == NULL) {
+    // If the ContextSwitcher thread is not running at the moment start it now.
+    singleton_ = new ContextSwitcher(every_n_ms);
+    singleton_->Start();
   } else {
-    switcher->sleep_ms_ = every_n_ms;
+    // ContextSwitcher thread is already running, so we just change the
+    // scheduling interval.
+    singleton_->sleep_ms_ = every_n_ms;
   }
 }
 
 
+// Disable preemption of V8 threads. If multiple threads want to use V8 they
+// must cooperatively schedule amongst them from this point on.
 void ContextSwitcher::StopPreemption() {
   ASSERT(Locker::IsLocked());
-  if (switcher != NULL) {
-    switcher->Stop();
-    delete(switcher);
-    switcher = NULL;
+  if (singleton_ != NULL) {
+    // The ContextSwitcher thread is running. We need to stop it and release
+    // its resources.
+    singleton_->keep_going_ = false;
+    singleton_->Join();  // Wait for the ContextSwitcher thread to exit.
+    // Thread has exited, now we can delete it.
+    delete(singleton_);
+    singleton_ = NULL;

[Erik Corry, 2008/12/17 14:30:07]

Perhaps here we should clear a pending preemption.  It might be confusing to
have it discovered after this point?

[iposva, 2008/12/17 17:39:24]

I disagree. To the running thread a pending preemption will appear like a
spurious interrupt, the thread has to be able to handle that anyway. Also you
would need to iterate through all the threads clearing all pending interrupts.

   }
 }
 
 
+// Main loop of the ContextSwitcher thread: Preempt the currently running V8
+// thread at regular intervals.
 void ContextSwitcher::Run() {
   while (keep_going_) {
     OS::Sleep(sleep_ms_);
     StackGuard::Preempt();
-    WaitForPreemption();
   }
 }
 
 
-void ContextSwitcher::Stop() {
+// Acknowledge the preemption by the receiving thread.
+void ContextSwitcher::PreemptionReceived() {
   ASSERT(Locker::IsLocked());
-  keep_going_ = false;
-  preemption_semaphore_->Signal();
-  Join();
+  // There is currently no accounting being done for this. But could be in the
+  // future, which is why we leave this in.
 }
 
 
-void ContextSwitcher::WaitForPreemption() {
-  preemption_semaphore_->Wait();
-}
-
-
-void ContextSwitcher::PreemptionReceived() {
-  ASSERT(Locker::IsLocked());
-  switcher->preemption_semaphore_->Signal();
-}
-
-
 }  // namespace internal
 }  // namespace v8