Fixed a bunch of memory leaks in tests, including:

- String traversal test data (now in a zone)
 - Debug message thread (now joined on exit)
 - Threading test threads (now joined on exit)
 - Changed message tests framework to cope with valgrind
Also, fixed a bug where we'd try to delete stack-allocated objects
when tearing down v8.  Good times.


git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1622 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

include/v8.h

@@ -1080,13 +1080,13 @@ class V8EXPORT Object : public Value {
    * access check info, the object cannot be accessed by anyone.
    */
   void TurnOnAccessCheck();
-  
+
   /**
    * Returns the identity hash for this object. The current implemenation uses
    * a hidden property on the object to store the identity hash.
    */
   int GetIdentityHash();
-  
+
   /**
    * Access hidden properties on JavaScript objects. These properties are
    * hidden from the executing JavaScript and only accessible through the V8
@@ -2033,6 +2033,17 @@ class V8EXPORT V8 {
    */
   static void ResumeProfiler();
 
+  /**
+   * Releases any resources used by v8 and stops any utility threads
+   * that may be running.  Note that disposing v8 is permanent, it
+   * cannot be reinitialized.
+   *
+   * It should generally not be necessary to dispose v8 before exiting
+   * a process, this should happen automatically.  It is only necessary
+   * to use if the process needs the resources taken up by v8.
+   */
+  static bool Dispose();
+
  private:
   V8();
 

samples/shell.cc

@@ -45,7 +45,7 @@ v8::Handle<v8::String> ReadFile(const char* name);
 void ReportException(v8::TryCatch* handler);
 
 
-int main(int argc, char* argv[]) {
+int RunMain(int argc, char* argv[]) {
   v8::V8::SetFlagsFromCommandLine(&argc, argv, true);
   v8::HandleScope handle_scope;
   // Create a template for the global object.
@@ -95,11 +95,17 @@ int main(int argc, char* argv[]) {
         return 1;
     }
   }
-  if (run_shell) RunShell(context);
   return 0;
 }
 
 
+int main(int argc, char* argv[]) {
+  int result = RunMain(argc, argv);
+  v8::V8::Dispose();
+  return result;
+}
+
+
 // Extracts a C string from a V8 Utf8Value.
 const char* ToCString(const v8::String::Utf8Value& value) {
   return *value ? *value : "<string conversion failed>";

src/api.cc

@@ -2272,6 +2272,12 @@ bool v8::V8::Initialize() {
 }
 
 
+bool v8::V8::Dispose() {
+  i::V8::TearDown();
+  return true;
+}
+
+
 const char* v8::V8::GetVersion() {
   return "1.1.4 (candidate)";
 }

src/debug.cc

@@ -1737,6 +1737,15 @@ void Debugger::SetEventListener(Handle<Object> callback,
 }
 
 
+void Debugger::TearDown() {
+  if (message_thread_ != NULL) {
+    message_thread_->Stop();
+    delete message_thread_;
+    message_thread_ = NULL;
+  }
+}
+
+
 void Debugger::SetMessageHandler(v8::DebugMessageHandler handler, void* data) {
   message_handler_ = handler;
   message_handler_data_ = data;
@@ -1852,16 +1861,23 @@ void Debugger::StopAgent() {
 DebugMessageThread::DebugMessageThread()
     : host_running_(true),
       command_queue_(kQueueInitialSize),
-      message_queue_(kQueueInitialSize) {
+      message_queue_(kQueueInitialSize),
+      keep_running_(true) {
   command_received_ = OS::CreateSemaphore(0);
   message_received_ = OS::CreateSemaphore(0);
 }
 
-// Does not free resources held by DebugMessageThread
-// because this cannot be done thread-safely.
+// Should only be done after the thread is done running.
 DebugMessageThread::~DebugMessageThread() {
+  delete command_received_;
+  delete message_received_;
 }
 
+void DebugMessageThread::Stop() {
+  keep_running_ = false;
+  SendMessage(Vector<uint16_t>(NULL, 0));
+  Join();
+}
 
 // Puts an event coming from V8 on the queue.  Creates
 // a copy of the JSON formatted event string managed by the V8.
@@ -1909,7 +1925,7 @@ bool DebugMessageThread::SetEventJSONFromEvent(Handle<Object> event_data) {
 
 void DebugMessageThread::Run() {
   // Sends debug events to an installed debugger message callback.
-  while (true) {
+  while (keep_running_) {
     // Wait and Get are paired so that semaphore count equals queue length.
     message_received_->Wait();
     Logger::DebugTag("Get message from event message_queue.");

src/debug.h

@@ -429,6 +429,7 @@ class Debugger {
                                 bool auto_continue);
   static void SetEventListener(Handle<Object> callback, Handle<Object> data);
   static void SetMessageHandler(v8::DebugMessageHandler handler, void* data);
+  static void TearDown();
   static void SetHostDispatchHandler(v8::DebugHostDispatchHandler handler,
                                      void* data);
   static void SendMessage(Vector<uint16_t> message);
@@ -527,7 +528,7 @@ class LockingMessageQueue BASE_EMBEDDED {
 class DebugMessageThread: public Thread {
  public:
   DebugMessageThread();  // Called from API thread.
-  virtual ~DebugMessageThread();  // Never called.
+  virtual ~DebugMessageThread();
   // Called by V8 thread.  Reports events from V8 VM.
   // Also handles command processing in stopped state of V8,
   // when host_running_ is false.
@@ -554,6 +555,7 @@ class DebugMessageThread: public Thread {
 
   // Check whether there are commands in the queue.
   bool HasCommands() { return !command_queue_.IsEmpty(); }
+  void Stop();
 
   bool host_running_;  // Is the debugging host running or stopped?
   Semaphore* command_received_;  // Non-zero when command queue is non-empty.
@@ -564,6 +566,7 @@ class DebugMessageThread: public Thread {
   static const int kQueueInitialSize = 4;
   LockingMessageQueue command_queue_;
   LockingMessageQueue message_queue_;
+  bool keep_running_;
   DISALLOW_COPY_AND_ASSIGN(DebugMessageThread);
 };
 

src/flag-definitions.h

@@ -62,7 +62,7 @@
 // printing / etc in the flag parser code.  We only do this for writable flags.
 #elif defined(FLAG_MODE_META)
 #define FLAG_FULL(ftype, ctype, nam, def, cmt) \
-  { Flag::TYPE_##ftype, #nam, &FLAG_##nam, &FLAGDEFAULT_##nam, cmt },
+  { Flag::TYPE_##ftype, #nam, &FLAG_##nam, &FLAGDEFAULT_##nam, cmt, false },
 #define FLAG_READONLY(ftype, ctype, nam, def, cmt)
 
 #else

src/flags.cc

@@ -58,6 +58,7 @@ struct Flag {
   void* valptr_;            // Pointer to the global flag variable.
   const void* defptr_;      // Pointer to the default value.
   const char* cmt_;         // A comment about the flags purpose.
+  bool owns_ptr_;           // Does the flag own its string value?
 
   FlagType type() const { return type_; }
 
@@ -80,9 +81,17 @@ struct Flag {
     return reinterpret_cast<double*>(valptr_);
   }
 
-  const char** string_variable() const {
+  const char* string_value() const {
     ASSERT(type_ == TYPE_STRING);
-    return reinterpret_cast<const char**>(valptr_);
+    return *reinterpret_cast<const char**>(valptr_);
+  }
+
+  void set_string_value(const char *value, bool owns_ptr) {
+    ASSERT(type_ == TYPE_STRING);
+    const char **ptr = reinterpret_cast<const char **>(valptr_);
+    if (owns_ptr_ && *ptr != NULL) DeleteArray(*ptr);
+    *ptr = value;
+    owns_ptr_ = owns_ptr;
   }
 
   JSArguments* args_variable() const {
@@ -125,7 +134,7 @@ struct Flag {
       case TYPE_FLOAT:
         return *float_variable() == float_default();
       case TYPE_STRING: {
-        const char* str1 = *string_variable();
+        const char* str1 = string_value();
         const char* str2 = string_default();
         if (str2 == NULL) return str1 == NULL;
         if (str1 == NULL) return str2 == NULL;
@@ -151,7 +160,7 @@ struct Flag {
         *float_variable() = float_default();
         break;
       case TYPE_STRING:
-        *string_variable() = string_default();
+        set_string_value(string_default(), false);
         break;
       case TYPE_ARGS:
         *args_variable() = args_default();
@@ -197,7 +206,7 @@ static SmartPointer<const char> ToString(Flag* flag) {
       buffer.Add("%f", FmtElm(*flag->float_variable()));
       break;
     case Flag::TYPE_STRING: {
-      const char* str = *flag->string_variable();
+      const char* str = flag->string_value();
       buffer.Add("%s", str ? str : "NULL");
       break;
     }
@@ -389,17 +398,17 @@ int FlagList::SetFlagsFromCommandLine(int* argc,
           *flag->float_variable() = strtod(value, &endp);
           break;
         case Flag::TYPE_STRING:
-          *flag->string_variable() = value;
+          flag->set_string_value(value ? StrDup(value) : NULL, true);
           break;
         case Flag::TYPE_ARGS: {
           int start_pos = (value == NULL) ? i : i - 1;
           int js_argc = *argc - start_pos;
           const char** js_argv = NewArray<const char*>(js_argc);
           if (value != NULL) {
-            js_argv[0] = value;
+            js_argv[0] = StrDup(value);
           }
           for (int k = i; k < *argc; k++) {
-            js_argv[k - start_pos] = argv[k];
+            js_argv[k - start_pos] = StrDup(argv[k]);
           }
           *flag->args_variable() = JSArguments(js_argc, js_argv);
           i = *argc;  // Consume all arguments
@@ -491,11 +500,7 @@ int FlagList::SetFlagsFromString(const char* str, int len) {
 
   // cleanup
   DeleteArray(argv);
-  // don't delete copy0 since the substrings
-  // may be pointed to by FLAG variables!
-  // (this is a memory leak, but it's minor since this
-  // code is only used for debugging, or perhaps once
-  // during initialization).
+  DeleteArray(copy0);
 
   return result;
 }

src/log.cc

@@ -407,6 +407,7 @@ FILE* Logger::logfile_ = NULL;
 Profiler* Logger::profiler_ = NULL;
 Mutex* Logger::mutex_ = NULL;
 VMState* Logger::current_state_ = NULL;
+VMState Logger::bottom_state_(OTHER);
 SlidingStateWindow* Logger::sliding_state_window_ = NULL;
 
 #endif  // ENABLE_LOGGING_AND_PROFILING
@@ -1017,7 +1018,7 @@ bool Logger::Setup() {
     mutex_ = OS::CreateMutex();
   }
 
-  current_state_ = new VMState(OTHER);
+  current_state_ = &bottom_state_;
 
   // as log is initialized early with V8, we can assume that JS execution
   // frames can never reach this point on stack
@@ -1052,8 +1053,6 @@ void Logger::TearDown() {
     profiler_ = NULL;
   }
 
-  // Deleting the current_state_ has the side effect of assigning to it(!).
-  while (current_state_) delete current_state_;
   delete sliding_state_window_;
 
   delete ticker_;

src/log.h

@@ -83,7 +83,7 @@ class LogMessageBuilder;
 #endif
 
 
-class VMState {
+class VMState BASE_EMBEDDED {
 #ifdef ENABLE_LOGGING_AND_PROFILING
  public:
   explicit VMState(StateTag state);
@@ -252,6 +252,9 @@ class Logger {
   // A stack of VM states.
   static VMState* current_state_;
 
+  // Singleton bottom or default vm state.
+  static VMState bottom_state_;
+
   // SlidingStateWindow instance keeping a sliding window of the most
   // recent VM states.
   static SlidingStateWindow* sliding_state_window_;

src/platform-linux.cc

@@ -417,6 +417,7 @@ class ThreadHandle::PlatformData : public Malloced {
       case ThreadHandle::INVALID: thread_ = kNoThread; break;
     }
   }
+
   pthread_t thread_;  // Thread handle for pthread.
 };
 

src/v8.cc

@@ -111,6 +111,8 @@ void V8::TearDown() {
   Heap::TearDown();
   Logger::TearDown();
 
+  Debugger::TearDown();
+
   has_been_setup_ = false;
   has_been_disposed_ = true;
 }

src/zone-inl.h

@@ -50,6 +50,12 @@ inline void* Zone::New(int size) {
 }
 
 
+template <typename T>
+T* Zone::NewArray(int length) {
+  return static_cast<T*>(Zone::New(length * sizeof(T)));
+}
+
+
 bool Zone::excess_allocation() {
   return segment_bytes_allocated_ > zone_excess_limit_;
 }

src/zone.h

@@ -58,6 +58,9 @@ class Zone {
   // allocating new segments of memory on demand using malloc().
   static inline void* New(int size);
 
+  template <typename T>
+  static inline T* NewArray(int length);
+
   // Delete all objects and free all memory allocated in the Zone.
   static void DeleteAll();
 

test/cctest/cctest.cc

@@ -30,6 +30,7 @@
 #include <cstring>
 #include <cstdio>
 #include "cctest.h"
+#include "debug.h"
 
 
 CcTest* CcTest::last_ = NULL;
@@ -120,5 +121,6 @@ int main(int argc, char* argv[]) {
   }
   if (print_run_count && tests_run != 1)
     printf("Ran %i tests.\n", tests_run);
+  v8::V8::Dispose();
   return 0;
 }

test/cctest/test-api.cc

@@ -154,7 +154,7 @@ class ApiTestFuzzer: public v8::internal::Thread {
 class RegisterThreadedTest {
  public:
   explicit RegisterThreadedTest(CcTest::TestFunction* callback)
-      : callback_(callback) {
+      : fuzzer_(NULL), callback_(callback) {
     prev_ = first_;
     first_ = this;
     count_++;
@@ -5098,6 +5098,10 @@ void ApiTestFuzzer::ContextSwitch() {
 
 void ApiTestFuzzer::TearDown() {
   fuzzing_ = false;
+  for (int i = 0; i < RegisterThreadedTest::count(); i++) {
+    ApiTestFuzzer *fuzzer = RegisterThreadedTest::nth(i)->fuzzer_;
+    if (fuzzer != NULL) fuzzer->Join();
+  }
 }
 
 

test/cctest/test-strings.cc

@@ -11,6 +11,7 @@
 
 #include "factory.h"
 #include "cctest.h"
+#include "zone-inl.h"
 
 unsigned int seed = 123;
 
@@ -48,6 +49,8 @@ static const int SUPER_DEEP_DEPTH = 80 * 1024;
 
 static void InitializeBuildingBlocks(
     Handle<String> building_blocks[NUMBER_OF_BUILDING_BLOCKS]) {
+  // A list of pointers that we don't have any interest in cleaning up.
+  // If they are reachable from a root then leak detection won't complain.
   for (int i = 0; i < NUMBER_OF_BUILDING_BLOCKS; i++) {
     int len = gen() % 16;
     if (len > 14) {
@@ -80,7 +83,7 @@ static void InitializeBuildingBlocks(
       }
       case 2: {
         class Resource: public v8::String::ExternalStringResource,
-                        public Malloced {
+                        public ZoneObject {
          public:
           explicit Resource(Vector<const uc16> string): data_(string.start()) {
             length_ = string.length();
@@ -92,7 +95,7 @@ static void InitializeBuildingBlocks(
           const uc16* data_;
           size_t length_;
         };
-        uc16* buf = NewArray<uc16>(len);
+        uc16* buf = Zone::NewArray<uc16>(len);
         for (int j = 0; j < len; j++) {
           buf[j] = gen() % 65536;
         }
@@ -212,6 +215,7 @@ TEST(Traverse) {
   InitializeVM();
   v8::HandleScope scope;
   Handle<String> building_blocks[NUMBER_OF_BUILDING_BLOCKS];
+  ZoneScope zone(DELETE_ON_EXIT);
   InitializeBuildingBlocks(building_blocks);
   Handle<String> flat = ConstructBalanced(building_blocks);
   FlattenString(flat);
@@ -303,6 +307,7 @@ TEST(Slice) {
   InitializeVM();
   v8::HandleScope scope;
   Handle<String> building_blocks[NUMBER_OF_BUILDING_BLOCKS];
+  ZoneScope zone(DELETE_ON_EXIT);
   InitializeBuildingBlocks(building_blocks);
 
   seed = 42;

test/message/testcfg.py

@@ -41,6 +41,11 @@ class MessageTestCase(test.TestCase):
     self.config = config
     self.mode = mode
 
+  def IgnoreLine(self, str):
+    """Ignore empty lines and valgrind output."""
+    if not str: return True
+    else: return str.startswith('==') or str.startswith('**')
+
   def IsFailureOutput(self, output):
     f = file(self.expected)
     # Skip initial '#' comment and spaces
@@ -58,7 +63,8 @@ class MessageTestCase(test.TestCase):
       pattern = '^%s$' % pattern
       patterns.append(pattern)
     # Compare actual output with the expected
-    outlines = [ s for s in output.stdout.split('\n') if s ]
+    raw_lines = output.stdout.split('\n')
+    outlines = [ s for s in raw_lines if not self.IgnoreLine(s) ]
     if len(outlines) != len(patterns):
       return True
     for i in xrange(len(patterns)):