cleanup memory management routines

- libxml has hooks for allocation/deallocation, no need to manually trigger
- remove dead code

src/html_parser.cc

@@ -1,4 +1,5 @@
 // Copyright 2009, Squish Tech, LLC.
+
 #include "xml_syntax_error.h"
 #include "html_parser.h"
 #include "html_document.h"

src/libxmljs.cc

@@ -1,4 +1,7 @@
 // Copyright 2009, Squish Tech, LLC.
+
+#include <v8.h>
+
 #include "libxmljs.h"
 #include "xml_syntax_error.h"
 #include "xml_document.h"
@@ -8,214 +11,119 @@
 
 namespace libxmljs {
 
-    v8::Persistent<v8::FunctionTemplate> memory_usage;
-
-namespace {
-
-void on_libxml_destruct(xmlNode* node) {
-}
+// ensure destruction at exit time
+// v8 doesn't cleanup its resources
+LibXMLJS LibXMLJS::instance;
 
-}  // namespace
+// track how much memory libxml2 is using
+int xml_memory_used = 0;
 
-LibXMLJS::LibXMLJS() {
-  xmlMemSetup(xmlMemFree, xmlMemMalloc, xmlMemRealloc, xmlMemoryStrdup);
+// wrapper for xmlMemMalloc to update v8's knowledge of memory used
+// the GC relies on this information
+void* xmlMemMallocWrap(size_t size) {
+    void* res = xmlMemMalloc(size);
 
-  xmlInitParser();  // Not always necessary, but necessary for thread safety.
-  // xmlRegisterNodeDefault(on_libxml_construct);
-  xmlDeregisterNodeDefault(on_libxml_destruct);
-  // xmlThrDefRegisterNodeDefault(on_libxml_construct);
-  xmlThrDefDeregisterNodeDefault(on_libxml_destruct);
-}
+    // no need to udpate memory if we didn't allocate
+    if (!res) {
+        return res;
+    }
 
-LibXMLJS::~LibXMLJS() {
-  xmlCleanupParser();  // As per xmlInitParser(), or memory leak will happen.
+    const int diff = xmlMemUsed() - xml_memory_used;
+    xml_memory_used += diff;
+    v8::V8::AdjustAmountOfExternalAllocatedMemory(diff);
+    return res;
 }
 
-LibXMLJS LibXMLJS::init_;
-
-static void
-OnFatalError(const char* location,
-             const char* message) {
-#define FATAL_ERROR "\033[1;31mV8 FATAL ERROR.\033[m"
-  if (location)
-    fprintf(stderr, FATAL_ERROR " %s %s\n", location, message);
-  else
-    fprintf(stderr, FATAL_ERROR " %s\n", message);
+// wrapper for xmlMemFree to update v8's knowledge of memory used
+// the GC relies on this information
+void xmlMemFreeWrap(void* p) {
+    xmlMemFree(p);
+
+    // if v8 is no longer running, don't try to adjust memory
+    // this happens when the v8 vm is shutdown and the program is exiting
+    // our cleanup routines for libxml will be called (freeing memory)
+    // but v8 is already offline and does not need to be informed
+    // trying to adjust after shutdown will result in a fatal error
+    if (v8::V8::IsDead()) {
+        return;
+    }
 
-  exit(1);
+    const int diff = xmlMemUsed() - xml_memory_used;
+    xml_memory_used += diff;
+    v8::V8::AdjustAmountOfExternalAllocatedMemory(diff);
 }
 
-// Extracts a C str from a V8 Utf8Value.
-// TODO: Fix this error state, maybe throw exception?
-const char *
-ToCString(const v8::String::Utf8Value& value) {
-  return *value ? *value : "<str conversion failed>";
-}
+// wrapper for xmlMemRealloc to update v8's knowledge of memory used
+void* xmlMemReallocWrap(void* ptr, size_t size) {
+    void* res = xmlMemRealloc(ptr, size);
 
-static void
-ReportException(v8::TryCatch* try_catch) {
-  v8::Handle<v8::Message> message = try_catch->Message();
-  if (message.IsEmpty()) {
-    fprintf(stderr, "Error: (no message)\n");
-    fflush(stderr);
-    return;
-  }
-  v8::Handle<v8::Value> error = try_catch->Exception();
-  v8::Handle<v8::String> stack;
-  if (error->IsObject()) {
-    v8::Handle<v8::Object> obj = v8::Handle<v8::Object>::Cast(error);
-    v8::Handle<v8::Value> raw_stack = obj->Get(v8::String::New("stack"));
-    if (raw_stack->IsString()) stack = v8::Handle<v8::String>::Cast(raw_stack);
-  }
-  if (stack.IsEmpty()) {
-    v8::String::Utf8Value exception(error);
-
-    // Print (filename):(line number): (message).
-    v8::String::Utf8Value filename(message->GetScriptResourceName());
-    const char* filename_string = ToCString(filename);
-    int linenum = message->GetLineNumber();
-    fprintf(stderr, "%s:%i: %s\n", filename_string, linenum, *exception);
-    // Print line of source code.
-    v8::String::Utf8Value sourceline(message->GetSourceLine());
-    const char* sourceline_string = ToCString(sourceline);
-    fprintf(stderr, "%s\n", sourceline_string);
-    // Print wavy underline (GetUnderline is deprecated).
-    int start = message->GetStartColumn();
-    for (int i = 0; i < start; i++) {
-      fprintf(stderr, " ");
+    // if realloc fails, no need to update v8 memory state
+    if (!res) {
+        return res;
     }
-    int end = message->GetEndColumn();
-    for (int i = start; i < end; i++) {
-      fprintf(stderr, "^");
-    }
-    fprintf(stderr, "\n");
-
-    message->PrintCurrentStackTrace(stderr);
-
-
-  } else {
-    v8::String::Utf8Value trace(stack);
-    fprintf(stderr, "%s\n", *trace);
-  }
-  fflush(stderr);
-}
-
-// Executes a str within the current v8 context.
-v8::Handle<v8::Value>
-ExecuteString(v8::Handle<v8::String> source,
-              v8::Handle<v8::Value> filename) {
-  v8::HandleScope scope;
-  v8::TryCatch try_catch;
-
-  v8::Handle<v8::Script> script = v8::Script::Compile(source, filename);
-  if (script.IsEmpty()) {
-    ReportException(&try_catch);
-    exit(1);
-  }
-
-  v8::Handle<v8::Value> result = script->Run();
-  if (result.IsEmpty()) {
-    ReportException(&try_catch);
-    exit(1);
-  }
-
-  return scope.Close(result);
-}
 
-void
-UpdateV8Memory() {
-    v8::V8::AdjustAmountOfExternalAllocatedMemory(-current_xml_memory);
-    current_xml_memory = xmlMemUsed();
-    v8::V8::AdjustAmountOfExternalAllocatedMemory(current_xml_memory);
+    const int diff = xmlMemUsed() - xml_memory_used;
+    xml_memory_used += diff;
+    v8::V8::AdjustAmountOfExternalAllocatedMemory(diff);
+    return res;
 }
 
-v8::Handle<v8::Value>
-MemoryUsage(const v8::Arguments& args) {
-    v8::HandleScope scope;
-    v8::Local<v8::Object> obj = v8::Object::New();
-    long c = xmlMemUsed();
+// wrapper for xmlMemoryStrdupWrap to update v8's knowledge of memory used
+char* xmlMemoryStrdupWrap(const char* str) {
+    char* res = xmlMemoryStrdup(str);
 
-    obj->Set(v8::String::New("libxml"), v8::Integer::New(c));
-    obj->Set(v8::String::New("v8"), v8::Integer::New(current_xml_memory));
+    // if strdup fails, no need to update v8 memory state
+    if (!res) {
+        return res;
+    }
 
-    return scope.Close(obj);
+    const int diff = xmlMemUsed() - xml_memory_used;
+    xml_memory_used += diff;
+    v8::V8::AdjustAmountOfExternalAllocatedMemory(diff);
+    return res;
 }
 
-void
-InitializeLibXMLJS(v8::Handle<v8::Object> target) {
-  v8::HandleScope scope;
-
-  XmlSyntaxError::Initialize(target);
-  XmlDocument::Initialize(target);
-  XmlParser::Initialize(target);
-  HtmlParser::Initialize(target);
-
-  target->Set(v8::String::NewSymbol("version"),
-              v8::String::New(LIBXMLJS_VERSION));
-
-  target->Set(v8::String::NewSymbol("libxml_version"),
-              v8::String::New(LIBXML_DOTTED_VERSION));
-
-  target->Set(v8::String::NewSymbol("libxml_parser_version"),
-              v8::String::New(xmlParserVersion));
-
-  target->Set(v8::String::NewSymbol("libxml_debug_enabled"),
-              v8::Boolean::New(debugging));
+LibXMLJS::LibXMLJS() {
 
-  v8::Local<v8::FunctionTemplate> func = v8::FunctionTemplate::New(MemoryUsage);
-  memory_usage = v8::Persistent<v8::FunctionTemplate>::New(func);
-  memory_usage->InstanceTemplate()->SetInternalFieldCount(1);
+    // populated debugMemSize (see xmlmemory.h/c) and makes the call to
+    // xmlMemUsed work, this must happen first!
+    xmlMemSetup(xmlMemFreeWrap, xmlMemMallocWrap,
+            xmlMemReallocWrap, xmlMemoryStrdupWrap);
 
-  target->Set(v8::String::NewSymbol("memoryUsage"),
-              memory_usage->GetFunction());
+    // initialize libxml
+    LIBXML_TEST_VERSION;
 
-  v8::Handle<v8::ObjectTemplate> global = v8::ObjectTemplate::New();
-  v8::Handle<v8::Context> context = v8::Context::New(NULL, global);
+    // initial memory usage
+    xml_memory_used = xmlMemUsed();
+}
 
-  v8::Context::Scope context_scope(context);
-  context->Global()->Set(v8::String::NewSymbol("libxml"), target);
+LibXMLJS::~LibXMLJS() {
+    xmlCleanupParser();
 }
 
 // used by node.js to initialize libraries
 extern "C" void
 init(v8::Handle<v8::Object> target) {
-  v8::HandleScope scope;
-  InitializeLibXMLJS(target);
-}
-
-int
-main(int argc,
-     char* argv[]) {
-  v8::V8::SetFlagsFromCommandLine(&argc, argv, true);
-  v8::V8::Initialize();
-  v8::V8::SetFatalErrorHandler(OnFatalError);
+      v8::HandleScope scope;
 
-  // Create a stack-allocated handle scope.
-  v8::HandleScope handle_scope;
-  // Create a new context.
-  v8::Handle<v8::Context> context = v8::Context::New();
-  // Enter the created context for compiling and
-  // running the hello world script.
-  v8::Context::Scope context_scope(context);
+      XmlSyntaxError::Initialize(target);
+      XmlDocument::Initialize(target);
+      XmlParser::Initialize(target);
+      HtmlParser::Initialize(target);
 
-  v8::Local<v8::Object> global_obj = v8::Context::GetCurrent()->Global();
-  v8::Local<v8::Object> libxml_obj = v8::Object::New();
+      target->Set(v8::String::NewSymbol("version"),
+                  v8::String::New(LIBXMLJS_VERSION));
 
-  InitializeLibXMLJS(libxml_obj);
+      target->Set(v8::String::NewSymbol("libxml_version"),
+                  v8::String::New(LIBXML_DOTTED_VERSION));
 
-  global_obj->Set(v8::String::NewSymbol("libxml"), libxml_obj);
+      target->Set(v8::String::NewSymbol("libxml_parser_version"),
+                  v8::String::New(xmlParserVersion));
 
-  // for (int i = 1; i < argc; i++) {
-  //   // Create a string containing the JavaScript source code.
-  //   Handle<String> source = ReadFile(argv[i]);
-  //   // Compile the source code.
-  //   Handle<Script> script = Script::Compile(source);
-  //   // Run the script to get the result.
-  //   Handle<Value> result = script->Run();
-  // }
+      target->Set(v8::String::NewSymbol("libxml_debug_enabled"),
+                  v8::Boolean::New(debugging));
 
-  v8::V8::Dispose();
-
-  return 0;
+      target->Set(v8::String::NewSymbol("libxml"), target);
 }
+
 }  // namespace libxmljs

src/libxmljs.h

@@ -16,9 +16,11 @@
 #include <libxml/HTMLtree.h>
 
 #include <memory>
-#include <string.h>
+#include <cstring>
 #include <cassert>  // for assert()
 
+#include "object_wrap.h"
+
 #define LIBXMLJS_VERSION "v0.4.0"
 
 #define BAD_ARGUMENTS Exception::TypeError(String::New("Bad argument"))
@@ -77,23 +79,16 @@ static const bool debugging = true;
 static const bool debugging = false;
 #endif
 
-    static long current_xml_memory = 0;
-    void UpdateV8Memory();
-
 // Ensure that libxml is properly initialised:
 class LibXMLJS {
-  public:
-
-  LibXMLJS();
-  virtual ~LibXMLJS();
+public:
+    LibXMLJS();
+    virtual ~LibXMLJS();
 
-  private:
-
-  static LibXMLJS init_;
+private:
+    static LibXMLJS instance;
 };
 
 }  // namespace libxmljs
 
-#include "object_wrap.h"
-
 #endif  // SRC_LIBXMLJS_H_

src/xml_attribute.cc

@@ -21,8 +21,6 @@ XmlAttribute::New(const v8::Arguments& args) {
                              (const xmlChar*)*name,
                              (const xmlChar*)*value);
 
-  UpdateV8Memory();
-
   // namespace passed in
   if (args.Length() == 4 && args[3]->IsObject()) {
     XmlNamespace *ns = LibXmlObj::Unwrap<XmlNamespace>(args[3]->ToObject());

src/xml_document.cc

@@ -136,8 +136,6 @@ XmlDocument::New(const v8::Arguments& args) {
 
   xmlDoc* doc = xmlNewDoc((const xmlChar*)**version);
 
-  UpdateV8Memory();
-
   v8::Handle<v8::Object> obj =
       LibXmlObj::GetMaybeBuild<XmlDocument, xmlDoc>(doc);
   XmlDocument *document = LibXmlObj::Unwrap<XmlDocument>(obj);
@@ -158,8 +156,7 @@ XmlDocument::New(const v8::Arguments& args) {
 }
 
 XmlDocument::~XmlDocument() {
-  xmlFree(xml_obj);
-  UpdateV8Memory();
+  xmlFreeDoc(xml_obj);
 }
 
 void

src/xml_element.cc

@@ -37,8 +37,6 @@ XmlElement::New(const v8::Arguments& args) {
                                 NULL,
                                 (const xmlChar*)*name,
                                 (const xmlChar*)content);
-  UpdateV8Memory();
-
   if(content)
       free(content);
 
@@ -481,8 +479,6 @@ XmlElement::import_element(XmlElement *element) {
         if(new_child == NULL) {
             return NULL;
         }
-
-        UpdateV8Memory();
 
         v8::Handle<v8::Object> obj =
             LibXmlObj::GetMaybeBuild<XmlElement, xmlNode>(new_child);