nodegit · srajko · Mar 23, 2016 · Mar 23, 2016 · Mar 24, 2016 · Mar 23, 2016
diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json
@@ -1767,6 +1767,8 @@
      "ignore": true
    },
    "repository": {
+      "cacheResult": true,
+      "oneToOne": true,
      "functions": {
        "git_repository__cleanup": {
          "ignore": true

diff --git a/generate/scripts/helpers.js b/generate/scripts/helpers.js
@@ -179,7 +179,9 @@ var Helpers = {
    typeDef.filename = typeDef.typeName;
    typeDef.isLibgitType = true;
    typeDef.dependencies = [];
+    typeDef.cacheResult = Boolean(typeDefOverrides.cacheResult);
    typeDef.selfFreeing = Boolean(typeDefOverrides.selfFreeing);
+    typeDef.oneToOne = Boolean(typeDefOverrides.oneToOne);

    if (typeDefOverrides.freeFunctionName) {
      typeDef.freeFunctionName = typeDefOverrides.freeFunctionName;

diff --git a/generate/templates/manual/include/functions/noop.h b/generate/templates/manual/include/functions/noop.h
@@ -0,0 +1,8 @@
+#ifndef NOOP_FUNCTIONS
+#define NOOP_FUNCTIONS
+
+#include <nan.h>
+
+void noop(const Nan::WeakCallbackInfo<void> &data);
+
+#endif
diff --git a/generate/templates/manual/src/functions/noop.cc b/generate/templates/manual/src/functions/noop.cc
@@ -0,0 +1,4 @@
+
+#include "../../include/functions/noop.h"
+
+void noop(const Nan::WeakCallbackInfo<void> &data) {}
diff --git a/generate/templates/partials/sync_function.cc b/generate/templates/partials/sync_function.cc
@@ -2,6 +2,11 @@
 {%partial doc .%}
 NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) {
  Nan::EscapableHandleScope scope;
+
+{%if return.cacheResult %}
+  {{ cppClassName }} *thisObj = Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This());
+  return info.GetReturnValue().Set(scope.Escape(Nan::New(thisObj->{{ cppFunctionName }}_cachedResult)));
+{% else %}
  {%partial guardArguments .%}

  {%each .|returnsInfo 'true' as _return %}
@@ -35,7 +40,7 @@ if (Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This())->GetValue() != NULL
 {% endif %}

  giterr_clear();
-  
+
  {
    LockMaster lockMaster(true{%each args|argsInfo as arg %}
      {%if arg.cType|isPointer%}{%if not arg.isReturn%}
@@ -125,4 +130,38 @@ if (Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This())->GetValue() != NULL
    {%endif%}
  {%endif%}
  }
+{%endif%}
 }
+
+{%if return.cacheResult %}
+void {{ cppClassName }}::{{ cppFunctionName }}_cache() {
+  if (!raw) {
+    {{ cppFunctionName }}_cachedResult.Reset(Nan::Null());
+    return;
+  }
+
+  LockMaster lockMaster(true{%each args|argsInfo as arg %}
+    {%if arg.cType|isPointer%}{%if not arg.isReturn%}
+      ,{%if arg.isSelf %}
+        raw
+    {%endif%}
+    {%endif%}{%endif%}
+  {%endeach%});
+
+  {{ return.cType }} result = {{ cFunctionName }}(
+  {%each args|argsInfo as arg %}
+    {%if arg.isSelf %}
+      raw
+    {%endif%}
+  {%endeach%}
+  );
+
+  Local<v8::Value> to;
+
+  {%each .|returnsInfo as _return %}
+    {%partial convertToV8 _return %}
+  {%endeach%}
+
+  {{ cppFunctionName }}_cachedResult.Reset(to);
+}
+{% endif %}
diff --git a/generate/templates/templates/binding.gyp b/generate/templates/templates/binding.gyp
@@ -18,6 +18,7 @@
        "src/promise_completion.cc",
        "src/wrapper.cc",
        "src/functions/copy.cc",
+        "src/functions/noop.cc",
        "src/functions/sleep_for_ms.cc",
        "src/convenient_patch.cc",
        "src/convenient_hunk.cc",

diff --git a/generate/templates/templates/class_content.cc b/generate/templates/templates/class_content.cc
@@ -10,6 +10,7 @@ extern "C" {

 #include "../include/lock_master.h"
 #include "../include/functions/copy.h"
+#include "../include/functions/noop.h"
 #include "../include/{{ filename }}.h"
 #include "../include/functions/sleep_for_ms.h"

@@ -25,6 +26,14 @@ using namespace node;

 {% if cType %}
  {{ cppClassName }}::{{ cppClassName }}({{ cType }} *raw, bool selfFreeing, bool shouldDuplicate) {
+    {% if oneToOne %}
+    if(instances.find(raw) != instances.end()) {
+      Nan::ThrowError("{{ cppClassName }} constructor called on an already wrapped {{ cType }} object");
+      return;
+    }
+    selfFreeing = true;
+    {% endif %}
+
    if (shouldDuplicate) {
      {% if shouldAlloc %}
    this->raw = ({{ cType }} *)malloc(sizeof({{ cType }}));
@@ -44,6 +53,16 @@ using namespace node;
      NonSelfFreeingConstructedCount++;
    }

+
+	{%each functions as function%}
+      {%if not function.ignore %}
+        {%if not function.isAsync %}
+          {%if function.return.cacheResult %}
+            {{ function.cppFunctionName }}_cache(); // populate cached value
+          {%endif%}
+        {%endif%}
+      {%endif%}
+    {%endeach%}
  }

  {{ cppClassName }}::~{{ cppClassName }}() {
@@ -52,7 +71,7 @@ using namespace node;
        {{ freeFunctionName }}(this->raw);
        SelfFreeingInstanceCount--;

-        this->raw = NULL;
+        ClearValue();
      }
    {% endif %}

@@ -126,8 +145,24 @@ using namespace node;

  Local<v8::Value> {{ cppClassName }}::New(const {{ cType }} *raw, bool selfFreeing, bool shouldDuplicate) {
    Nan::EscapableHandleScope scope;
+
+    {% if oneToOne %}
+    auto mapElement = instances.find(const_cast<{{ cType }} *>(raw));
+    if(mapElement != instances.end()) {
+      return scope.Escape(Nan::New(*mapElement->second));
+    }
+    {% endif %}
+
    Local<v8::Value> argv[3] = { Nan::New<External>((void *)raw), Nan::New(selfFreeing), Nan::New(shouldDuplicate) };
-    return scope.Escape(Nan::NewInstance(Nan::New({{ cppClassName }}::constructor_template), 3, argv).ToLocalChecked());
+    Local<v8::Object> instance = Nan::NewInstance(Nan::New({{ cppClassName }}::constructor_template), 3, argv).ToLocalChecked();
+    {% if oneToOne %}
+    Nan::Persistent<v8::Object> *persistent = new Nan::Persistent<v8::Object>(instance);
+    persistent->SetWeak((void *)NULL, noop, Nan::WeakCallbackType::kParameter);
+    if (raw) {
+      instances[const_cast<git_repository *>(raw)] = persistent;
+    }
+    {% endif %}
+    return scope.Escape(instance);
  }

  NAN_METHOD({{ cppClassName }}::GetSelfFreeingInstanceCount) {
@@ -143,6 +178,14 @@ using namespace node;
  }

  void {{ cppClassName }}::ClearValue() {
+    {% if oneToOne %}
+    auto mapElement = instances.find(raw);
+    if(mapElement != instances.end()) {
+      delete mapElement->second;
+      instances.erase(mapElement);
+    }
+    {% endif %}
+
    this->raw = NULL;
  }

@@ -182,5 +225,10 @@ using namespace node;
  Nan::Persistent<Function> {{ cppClassName }}::constructor_template;
 {% endif %}

+{%if oneToOne %}
+// map to ensure one wrapper per libgit2 instance
+std::map<{{ cType }} *, Nan::Persistent<v8::Object> *> {{ cppClassName }}::instances;
+{%endif%}
+
 int {{ cppClassName }}::SelfFreeingInstanceCount;
 int {{ cppClassName }}::NonSelfFreeingConstructedCount;
diff --git a/generate/templates/templates/class_header.h b/generate/templates/templates/class_header.h
@@ -1,6 +1,7 @@
 #ifndef {{ cppClassName|upper }}_H
 #define {{ cppClassName|upper }}_H
 #include <nan.h>
+#include <map>
 #include <string>
 #include <queue>
 #include <utility>
@@ -36,6 +37,10 @@ using namespace node;
 using namespace v8;

 class {{ cppClassName }} : public Nan::ObjectWrap {
+    {%if oneToOne %}
+    // map to ensure one wrapper per libgit2 instance
+    static std::map<{{ cType }} *, Nan::Persistent<v8::Object> *> instances;
+    {%endif%}
  public:

    static Nan::Persistent<Function> constructor_template;
@@ -141,6 +146,14 @@ class {{ cppClassName }} : public Nan::ObjectWrap {
      private:
        {{ function.cppFunctionName }}Baton *baton;
    };
+        {%else%}
+    {%if function.return.cacheResult %}
+    // For simple sync functions that return a wrapped object and pass `raw`
+    // as the the only parameter to libgit2, we cache the results.
+    // CopyablePersistentTraits are used to get the reset-on-destruct behavior.
+    void {{ function.cppFunctionName }}_cache();
+    Nan::Persistent<Value, Nan::CopyablePersistentTraits<Value> > {{ function.cppFunctionName }}_cachedResult;
+    {%endif%}
        {%endif%}

    static NAN_METHOD({{ function.cppFunctionName }});

diff --git a/test/tests/commit.js b/test/tests/commit.js
@@ -404,6 +404,38 @@ describe("Commit", function() {
    assert.ok(owner instanceof Repository);
  });

+  it("caches its owner", function(done) {
+    var objects = {};
+
+    reinitialize(objects)
+      .then(function() {
+        var owner = objects.commit.owner();
+        var ownerAgain = objects.commit.owner();
+        assert.ok(owner === ownerAgain);
+        assert.ok(owner === objects.repository);
+
+        // make sure the owner gets freed at the correct time
+        garbageCollect();
+        var Repository = NodeGit.Repository;
+        var startCount = Repository.getSelfFreeingInstanceCount();
+        owner = ownerAgain = objects.repository = null;
+
+        setTimeout(function() {
+          garbageCollect();
+          // the commit should still hold the repository
+          assert.equal(startCount, Repository.getSelfFreeingInstanceCount());
+          objects.commit = null;
+          // without the setTimeout, not seeing a free
+          setTimeout(function() {
+            garbageCollect();
+            assert.equal(startCount - 1,
+              Repository.getSelfFreeingInstanceCount());
+            done();
+          }, 10);
+        }, 10);
+      });
+  });
+
  it("can walk its repository's history", function(done) {
    var historyCount = 0;
    var expectedHistoryCount = 364;

diff --git a/test/tests/submodule.js b/test/tests/submodule.js
@@ -134,7 +134,7 @@ describe("Submodule", function() {
        return reference.peel(NodeGit.Object.TYPE.COMMIT);
      })
      .then(function(commit) {
-        return submoduleRepo.createBranch("master", commit);
+        return submoduleRepo.createBranch("master", commit.id());
      })
      .then(function() {
        return submodule.addFinalize();
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,4 @@

		#include "../../include/functions/noop.h"

		void noop(const Nan::WeakCallbackInfo<void> &data) {}