From 88d83457efeae850a0f0da63f53925c13c09ca4c Mon Sep 17 00:00:00 2001 From: Tom Rathbone Date: Tue, 7 May 2013 16:50:35 +0100 Subject: [PATCH 1/4] changes from the JavascriptFunction request made on codeplex --- .../Noesis.Javascript/JavascriptFunction.cpp | 70 +++++++++++++++++++ Source/Noesis.Javascript/JavascriptFunction.h | 47 +++++++++++++ .../Noesis.Javascript/JavascriptInterop.cpp | 3 + .../Noesis.Javascript.VS2012.vcxproj | 2 + .../Noesis.Javascript.VS2012.vcxproj.filters | 8 ++- .../Noesis.Javascript.Tests.VS2012.csproj | 1 + .../RegressionTests_JavascriptFunction.cs | 30 ++++++++ 7 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 Source/Noesis.Javascript/JavascriptFunction.cpp create mode 100644 Source/Noesis.Javascript/JavascriptFunction.h create mode 100644 Tests/Noesis.Javascript.Tests/RegressionTests/RegressionTests_JavascriptFunction.cs diff --git a/Source/Noesis.Javascript/JavascriptFunction.cpp b/Source/Noesis.Javascript/JavascriptFunction.cpp new file mode 100644 index 0000000..f66a555 --- /dev/null +++ b/Source/Noesis.Javascript/JavascriptFunction.cpp @@ -0,0 +1,70 @@ +#include "JavascriptFunction.h" +#include "JavascriptInterop.h" + +//////////////////////////////////////////////////////////////////////////////////////////////////// + +namespace Noesis { namespace Javascript { + +//////////////////////////////////////////////////////////////////////////////////////////////////// + +//////////////////////////////////////////////////////////////////////////////////////////////////// + +JavascriptFunction::JavascriptFunction( v8::Handle iFunction) +{ + if (!iFunction->IsFunction()) + throw gcnew System::ArgumentException("Trying to use non-function as function"); + + mFuncHandle = new Persistent(Handle::Cast(iFunction)); +} + +JavascriptFunction::~JavascriptFunction() +{ + mFuncHandle->Dispose(); + delete mFuncHandle; +} + +JavascriptFunction::!JavascriptFunction() +{ + mFuncHandle->Dispose(); + delete mFuncHandle; +} + +System::Object^ JavascriptFunction::Call(... cli::array^ args) +{ + Handle global = (*mFuncHandle)->CreationContext()->Global(); + + int argc = args->Length; + Handle *argv = new Handle[argc]; + for (int i = 0; i < argc; i++) + { + argv[i] = JavascriptInterop::ConvertToV8(args[i]); + } + + Local retVal = (*mFuncHandle)->Call(global, argc, argv); + + delete [] argv; + return JavascriptInterop::ConvertFromV8(retVal); +} + +bool JavascriptFunction::operator==( JavascriptFunction^ func1, JavascriptFunction^ func2 ) +{ + Handle jsFuncPtr1 = *(func1->mFuncHandle); + Handle jsFuncPtr2 = *(func2->mFuncHandle); + + return jsFuncPtr1->Equals(jsFuncPtr2); +} + +bool JavascriptFunction::Equals( JavascriptFunction^ other ) +{ + return this == other; +} + +bool JavascriptFunction::Equals(Object^ other ) +{ + JavascriptFunction^ otherFunc = safe_cast(other); + return (otherFunc && this->Equals(otherFunc)); +} + +} } // namespace Noesis::Javascript + +//////////////////////////////////////////////////////////////////////////////////////////////////// \ No newline at end of file diff --git a/Source/Noesis.Javascript/JavascriptFunction.h b/Source/Noesis.Javascript/JavascriptFunction.h new file mode 100644 index 0000000..23ca7d9 --- /dev/null +++ b/Source/Noesis.Javascript/JavascriptFunction.h @@ -0,0 +1,47 @@ +#pragma once + +////////////////////////////////////////////////////////////////////////// + +#include + +#include "JavascriptContext.h" + +using namespace v8; + +////////////////////////////////////////////////////////////////////////// + +namespace Noesis { namespace Javascript { + +////////////////////////////////////////////////////////////////////////// + +////////////////////////////////////////////////////////////////////////// +// JavascriptFunction +// +// Wraps around JS function object and allow calling it in later time +////////////////////////////////////////////////////////////////////////// +public ref class JavascriptFunction +{ +public: + JavascriptFunction(v8::Handle iFunction); + ~JavascriptFunction(); + !JavascriptFunction(); + + System::Object^ Call(... cli::array^ args); + + static bool operator== (JavascriptFunction^ func1, JavascriptFunction^ func2); + bool Equals(JavascriptFunction^ other); + + virtual bool Equals(Object^ other) override; + +private: + v8::Persistent* mFuncHandle; + +private: + JavascriptFunction() {} +}; + +////////////////////////////////////////////////////////////////////////// + +} } // namespace Noesis::Javascript + +////////////////////////////////////////////////////////////////////////// \ No newline at end of file diff --git a/Source/Noesis.Javascript/JavascriptInterop.cpp b/Source/Noesis.Javascript/JavascriptInterop.cpp index ebfcfcf..bafb4c3 100644 --- a/Source/Noesis.Javascript/JavascriptInterop.cpp +++ b/Source/Noesis.Javascript/JavascriptInterop.cpp @@ -33,6 +33,7 @@ #include "SystemInterop.h" #include "JavascriptException.h" #include "JavascriptExternal.h" +#include "JavascriptFunction.h" #include @@ -81,6 +82,8 @@ JavascriptInterop::ConvertFromV8(Handle iValue) return ConvertArrayFromV8(iValue); if (iValue->IsDate()) return ConvertDateFromV8(iValue); + if (iValue->IsFunction()) + return gcnew JavascriptFunction(iValue->ToObject()); if (iValue->IsObject()) { Handle object = iValue->ToObject(); diff --git a/Source/Noesis.Javascript/Noesis.Javascript.VS2012.vcxproj b/Source/Noesis.Javascript/Noesis.Javascript.VS2012.vcxproj index 1b7928d..c751847 100644 --- a/Source/Noesis.Javascript/Noesis.Javascript.VS2012.vcxproj +++ b/Source/Noesis.Javascript/Noesis.Javascript.VS2012.vcxproj @@ -185,6 +185,7 @@ + @@ -192,6 +193,7 @@ + diff --git a/Source/Noesis.Javascript/Noesis.Javascript.VS2012.vcxproj.filters b/Source/Noesis.Javascript/Noesis.Javascript.VS2012.vcxproj.filters index c3c8574..726ee0a 100644 --- a/Source/Noesis.Javascript/Noesis.Javascript.VS2012.vcxproj.filters +++ b/Source/Noesis.Javascript/Noesis.Javascript.VS2012.vcxproj.filters @@ -22,7 +22,10 @@ Source Files - + + Source Files + + Source Files @@ -42,6 +45,9 @@ Source Files + + Source Files + diff --git a/Tests/Noesis.Javascript.Tests/Noesis.Javascript.Tests.VS2012.csproj b/Tests/Noesis.Javascript.Tests/Noesis.Javascript.Tests.VS2012.csproj index 849c6b4..548ed72 100644 --- a/Tests/Noesis.Javascript.Tests/Noesis.Javascript.Tests.VS2012.csproj +++ b/Tests/Noesis.Javascript.Tests/Noesis.Javascript.Tests.VS2012.csproj @@ -106,6 +106,7 @@ + diff --git a/Tests/Noesis.Javascript.Tests/RegressionTests/RegressionTests_JavascriptFunction.cs b/Tests/Noesis.Javascript.Tests/RegressionTests/RegressionTests_JavascriptFunction.cs new file mode 100644 index 0000000..4a50895 --- /dev/null +++ b/Tests/Noesis.Javascript.Tests/RegressionTests/RegressionTests_JavascriptFunction.cs @@ -0,0 +1,30 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Diagnostics; + +namespace Noesis.Javascript.Tests +{ + public partial class RegressionTests + { + public static string GetFunctionFromJsContext(string js_dir) + { + JavascriptTest javascriptTest = new JavascriptTest(); + JavascriptContext context = new JavascriptContext(); + context.Run("var a = function(a,b) {return a+b;}"); + + JavascriptFunction funcObj = context.GetParameter("a") as JavascriptFunction; + + javascriptTest.Assert(funcObj != null); + + object result = funcObj.Call(1, 2); + + javascriptTest.Assert(result is int); + javascriptTest.Assert((int)result == 3); + + return null; + } + + } +} From 455771c656ac289cfdbfcb5fc022d71d9146b7c1 Mon Sep 17 00:00:00 2001 From: Tom Rathbone Date: Wed, 8 May 2013 16:09:39 +0100 Subject: [PATCH 2/4] make JavascriptFunction enter JavascriptContext before calling apply still throws AccessViolationException --- Source/Noesis.Javascript/JavascriptContext.h | 7 +++++-- .../Noesis.Javascript/JavascriptFunction.cpp | 19 +++++++++++++++++-- Source/Noesis.Javascript/JavascriptFunction.h | 3 ++- .../Noesis.Javascript/JavascriptInterop.cpp | 2 +- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/Source/Noesis.Javascript/JavascriptContext.h b/Source/Noesis.Javascript/JavascriptContext.h index 654f77d..a0b05cd 100644 --- a/Source/Noesis.Javascript/JavascriptContext.h +++ b/Source/Noesis.Javascript/JavascriptContext.h @@ -115,13 +115,16 @@ public ref class JavascriptContext: public System::IDisposable //////////////////////////////////////////////////////////// // Data members //////////////////////////////////////////////////////////// + + // v8 context required to be active for all v8 operations. + Persistent* mContext; + protected: // By entering an isolate before using a context, we can have multiple // contexts used simultaneously in different threads. v8::Isolate *isolate; - // v8 context required to be active for all v8 operations. - Persistent* mContext; + // v8 objects we hang onto for the duration. vector* mExternals; diff --git a/Source/Noesis.Javascript/JavascriptFunction.cpp b/Source/Noesis.Javascript/JavascriptFunction.cpp index f66a555..c653b8d 100644 --- a/Source/Noesis.Javascript/JavascriptFunction.cpp +++ b/Source/Noesis.Javascript/JavascriptFunction.cpp @@ -1,5 +1,6 @@ #include "JavascriptFunction.h" #include "JavascriptInterop.h" +#include "JavascriptContext.h" //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -9,12 +10,16 @@ namespace Noesis { namespace Javascript { //////////////////////////////////////////////////////////////////////////////////////////////////// -JavascriptFunction::JavascriptFunction( v8::Handle iFunction) +JavascriptFunction::JavascriptFunction( v8::Handle iFunction, JavascriptContext^ context) { if (!iFunction->IsFunction()) throw gcnew System::ArgumentException("Trying to use non-function as function"); + if(!context) + throw gcnew System::ArgumentException("Must provide a JavascriptContext"); + mFuncHandle = new Persistent(Handle::Cast(iFunction)); + mContext = context; } JavascriptFunction::~JavascriptFunction() @@ -31,7 +36,14 @@ JavascriptFunction::!JavascriptFunction() System::Object^ JavascriptFunction::Call(... cli::array^ args) { + JavascriptScope scope(mContext); + HandleScope handleScope(); + + // Context::Scope contextScope(*mContext->mContext); + // Handle global = (*mContext->mContext)->Global(); + Handle global = (*mFuncHandle)->CreationContext()->Global(); + int argc = args->Length; Handle *argv = new Handle[argc]; @@ -48,6 +60,9 @@ System::Object^ JavascriptFunction::Call(... cli::array^ args) bool JavascriptFunction::operator==( JavascriptFunction^ func1, JavascriptFunction^ func2 ) { + if(func2 == nullptr) { + return false; + } Handle jsFuncPtr1 = *(func1->mFuncHandle); Handle jsFuncPtr2 = *(func2->mFuncHandle); @@ -61,7 +76,7 @@ bool JavascriptFunction::Equals( JavascriptFunction^ other ) bool JavascriptFunction::Equals(Object^ other ) { - JavascriptFunction^ otherFunc = safe_cast(other); + JavascriptFunction^ otherFunc = dynamic_cast(other); return (otherFunc && this->Equals(otherFunc)); } diff --git a/Source/Noesis.Javascript/JavascriptFunction.h b/Source/Noesis.Javascript/JavascriptFunction.h index 23ca7d9..731ab19 100644 --- a/Source/Noesis.Javascript/JavascriptFunction.h +++ b/Source/Noesis.Javascript/JavascriptFunction.h @@ -22,7 +22,7 @@ namespace Noesis { namespace Javascript { public ref class JavascriptFunction { public: - JavascriptFunction(v8::Handle iFunction); + JavascriptFunction(v8::Handle iFunction, JavascriptContext^ context); ~JavascriptFunction(); !JavascriptFunction(); @@ -35,6 +35,7 @@ public ref class JavascriptFunction private: v8::Persistent* mFuncHandle; + JavascriptContext^ mContext; private: JavascriptFunction() {} diff --git a/Source/Noesis.Javascript/JavascriptInterop.cpp b/Source/Noesis.Javascript/JavascriptInterop.cpp index bafb4c3..c9e634a 100644 --- a/Source/Noesis.Javascript/JavascriptInterop.cpp +++ b/Source/Noesis.Javascript/JavascriptInterop.cpp @@ -83,7 +83,7 @@ JavascriptInterop::ConvertFromV8(Handle iValue) if (iValue->IsDate()) return ConvertDateFromV8(iValue); if (iValue->IsFunction()) - return gcnew JavascriptFunction(iValue->ToObject()); + return gcnew JavascriptFunction(iValue->ToObject(), JavascriptContext::GetCurrent()); if (iValue->IsObject()) { Handle object = iValue->ToObject(); From cc7c23f12f9cd2ee745b7fce3a646a1dd20ed50b Mon Sep 17 00:00:00 2001 From: Tom Rathbone Date: Mon, 13 May 2013 15:52:20 +0100 Subject: [PATCH 3/4] fix up JavascriptFunction --- Source/Noesis.Javascript/JavascriptContext.h | 6 ++-- .../Noesis.Javascript/JavascriptFunction.cpp | 32 ++++++++++++------- Source/Noesis.Javascript/JavascriptFunction.h | 5 +-- .../RegressionTests_JavascriptFunction.cs | 2 +- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/Source/Noesis.Javascript/JavascriptContext.h b/Source/Noesis.Javascript/JavascriptContext.h index a0b05cd..0bcc7cc 100644 --- a/Source/Noesis.Javascript/JavascriptContext.h +++ b/Source/Noesis.Javascript/JavascriptContext.h @@ -116,15 +116,13 @@ public ref class JavascriptContext: public System::IDisposable // Data members //////////////////////////////////////////////////////////// - // v8 context required to be active for all v8 operations. - Persistent* mContext; - protected: // By entering an isolate before using a context, we can have multiple // contexts used simultaneously in different threads. v8::Isolate *isolate; - + // v8 context required to be active for all v8 operations. + Persistent* mContext; // v8 objects we hang onto for the duration. vector* mExternals; diff --git a/Source/Noesis.Javascript/JavascriptFunction.cpp b/Source/Noesis.Javascript/JavascriptFunction.cpp index c653b8d..94ac8dc 100644 --- a/Source/Noesis.Javascript/JavascriptFunction.cpp +++ b/Source/Noesis.Javascript/JavascriptFunction.cpp @@ -18,32 +18,40 @@ JavascriptFunction::JavascriptFunction( v8::Handle iFunction, Javasc if(!context) throw gcnew System::ArgumentException("Must provide a JavascriptContext"); - mFuncHandle = new Persistent(Handle::Cast(iFunction)); + mFuncHandle = new Persistent(); + *mFuncHandle = Persistent::New(Handle::Cast(iFunction)); mContext = context; } JavascriptFunction::~JavascriptFunction() { - mFuncHandle->Dispose(); - delete mFuncHandle; + if(mFuncHandle) + { + JavascriptScope scope(mContext); + mFuncHandle->Dispose(); + delete mFuncHandle; + mFuncHandle = nullptr; + } + System::GC::SuppressFinalize(this); } JavascriptFunction::!JavascriptFunction() { - mFuncHandle->Dispose(); - delete mFuncHandle; + if(mFuncHandle) + { + JavascriptScope scope(mContext); + mFuncHandle->Dispose(); + delete mFuncHandle; + mFuncHandle = nullptr; + } } System::Object^ JavascriptFunction::Call(... cli::array^ args) -{ +{ JavascriptScope scope(mContext); - HandleScope handleScope(); - - // Context::Scope contextScope(*mContext->mContext); - // Handle global = (*mContext->mContext)->Global(); + HandleScope handleScope; Handle global = (*mFuncHandle)->CreationContext()->Global(); - int argc = args->Length; Handle *argv = new Handle[argc]; @@ -60,7 +68,7 @@ System::Object^ JavascriptFunction::Call(... cli::array^ args) bool JavascriptFunction::operator==( JavascriptFunction^ func1, JavascriptFunction^ func2 ) { - if(func2 == nullptr) { + if(ReferenceEquals(func2, nullptr)) { return false; } Handle jsFuncPtr1 = *(func1->mFuncHandle); diff --git a/Source/Noesis.Javascript/JavascriptFunction.h b/Source/Noesis.Javascript/JavascriptFunction.h index 731ab19..387f03c 100644 --- a/Source/Noesis.Javascript/JavascriptFunction.h +++ b/Source/Noesis.Javascript/JavascriptFunction.h @@ -35,10 +35,7 @@ public ref class JavascriptFunction private: v8::Persistent* mFuncHandle; - JavascriptContext^ mContext; - -private: - JavascriptFunction() {} + JavascriptContext^ mContext; }; ////////////////////////////////////////////////////////////////////////// diff --git a/Tests/Noesis.Javascript.Tests/RegressionTests/RegressionTests_JavascriptFunction.cs b/Tests/Noesis.Javascript.Tests/RegressionTests/RegressionTests_JavascriptFunction.cs index 4a50895..8411e0b 100644 --- a/Tests/Noesis.Javascript.Tests/RegressionTests/RegressionTests_JavascriptFunction.cs +++ b/Tests/Noesis.Javascript.Tests/RegressionTests/RegressionTests_JavascriptFunction.cs @@ -12,7 +12,7 @@ public static string GetFunctionFromJsContext(string js_dir) { JavascriptTest javascriptTest = new JavascriptTest(); JavascriptContext context = new JavascriptContext(); - context.Run("var a = function(a,b) {return a+b;}"); + context.Run("a = function(a,b) {return a+b;}"); JavascriptFunction funcObj = context.GetParameter("a") as JavascriptFunction; From 2890fe3fef56cd4345a2458473992b6b9f4e5ced Mon Sep 17 00:00:00 2001 From: Tom Rathbone Date: Mon, 13 May 2013 16:00:36 +0100 Subject: [PATCH 4/4] rebase and fix up --- .../Noesis.Javascript.Tests/IsolationTests.cs | 4 +-- .../JavascriptFunctionTests.cs | 23 ++++++++++++++ .../MemoryLeakTests.cs | 6 ++-- .../Noesis.Javascript.Tests.VS2012.csproj | 3 +- .../RegressionTests_JavascriptFunction.cs | 30 ------------------- 5 files changed, 28 insertions(+), 38 deletions(-) create mode 100644 Tests/Noesis.Javascript.Tests/JavascriptFunctionTests.cs delete mode 100644 Tests/Noesis.Javascript.Tests/RegressionTests/RegressionTests_JavascriptFunction.cs diff --git a/Tests/Noesis.Javascript.Tests/IsolationTests.cs b/Tests/Noesis.Javascript.Tests/IsolationTests.cs index e533fa0..ad43f2f 100644 --- a/Tests/Noesis.Javascript.Tests/IsolationTests.cs +++ b/Tests/Noesis.Javascript.Tests/IsolationTests.cs @@ -12,7 +12,7 @@ namespace Noesis.Javascript.Tests public class IsolationTests { [Test] - public string RunIsolatesTest() + public void RunIsolatesTest() { Stopwatch timer = new Stopwatch(); @@ -24,8 +24,6 @@ public string RunIsolatesTest() thread.Join(); Assert.That(timer.ElapsedMilliseconds, Is.LessThan(1500), "It took too long, they must not be running in parallel."); - - return null; } static void RunInstance() diff --git a/Tests/Noesis.Javascript.Tests/JavascriptFunctionTests.cs b/Tests/Noesis.Javascript.Tests/JavascriptFunctionTests.cs new file mode 100644 index 0000000..359f3fc --- /dev/null +++ b/Tests/Noesis.Javascript.Tests/JavascriptFunctionTests.cs @@ -0,0 +1,23 @@ +using NUnit.Framework; + +namespace Noesis.Javascript.Tests +{ + [TestFixture] + public class JavascriptFunctionTests + { + [Test] + public void GetFunctionFromJsContext() + { + JavascriptContext context = new JavascriptContext(); + context.Run("a = function(a,b) {return a+b;}"); + + JavascriptFunction funcObj = context.GetParameter("a") as JavascriptFunction; + + Assert.That(funcObj, Is.Not.Null); + + object result = funcObj.Call(1, 2); + + Assert.That(result, Is.EqualTo(3)); + } + } +} diff --git a/Tests/Noesis.Javascript.Tests/MemoryLeakTests.cs b/Tests/Noesis.Javascript.Tests/MemoryLeakTests.cs index e4d8f2a..a9fb6df 100644 --- a/Tests/Noesis.Javascript.Tests/MemoryLeakTests.cs +++ b/Tests/Noesis.Javascript.Tests/MemoryLeakTests.cs @@ -8,7 +8,7 @@ namespace Noesis.Javascript.Tests public class MemoryLeakTests { [Test] - public string RunMemoryLeakTest() + public void RunMemoryLeakTest() { MemoryUsageLoadInstance(); long mem = Process.GetCurrentProcess().PrivateMemorySize64; @@ -21,9 +21,7 @@ public string RunMemoryLeakTest() GC.Collect(); decimal diffMBytes = (Process.GetCurrentProcess().PrivateMemorySize64 - mem) / 1048576m; - if (diffMBytes >= 1) // Allow 1 MB - return String.Format("{0:0.00}MB left allocated", diffMBytes); - return null; + Assert.That(diffMBytes, Is.LessThan(1), "{0:0.00}MB left allocated", diffMBytes); } private static void MemoryUsageLoadInstance() diff --git a/Tests/Noesis.Javascript.Tests/Noesis.Javascript.Tests.VS2012.csproj b/Tests/Noesis.Javascript.Tests/Noesis.Javascript.Tests.VS2012.csproj index 548ed72..8c24d37 100644 --- a/Tests/Noesis.Javascript.Tests/Noesis.Javascript.Tests.VS2012.csproj +++ b/Tests/Noesis.Javascript.Tests/Noesis.Javascript.Tests.VS2012.csproj @@ -106,7 +106,7 @@ - + @@ -152,6 +152,7 @@ + diff --git a/Tests/Noesis.Javascript.Tests/RegressionTests/RegressionTests_JavascriptFunction.cs b/Tests/Noesis.Javascript.Tests/RegressionTests/RegressionTests_JavascriptFunction.cs deleted file mode 100644 index 8411e0b..0000000 --- a/Tests/Noesis.Javascript.Tests/RegressionTests/RegressionTests_JavascriptFunction.cs +++ /dev/null @@ -1,30 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Diagnostics; - -namespace Noesis.Javascript.Tests -{ - public partial class RegressionTests - { - public static string GetFunctionFromJsContext(string js_dir) - { - JavascriptTest javascriptTest = new JavascriptTest(); - JavascriptContext context = new JavascriptContext(); - context.Run("a = function(a,b) {return a+b;}"); - - JavascriptFunction funcObj = context.GetParameter("a") as JavascriptFunction; - - javascriptTest.Assert(funcObj != null); - - object result = funcObj.Call(1, 2); - - javascriptTest.Assert(result is int); - javascriptTest.Assert((int)result == 3); - - return null; - } - - } -}