diff --git a/Source/Noesis.Javascript/JavascriptContext.cpp b/Source/Noesis.Javascript/JavascriptContext.cpp index 789d3a8..b3203f1 100644 --- a/Source/Noesis.Javascript/JavascriptContext.cpp +++ b/Source/Noesis.Javascript/JavascriptContext.cpp @@ -159,8 +159,8 @@ JavascriptContext::JavascriptContext() isolate->SetFatalErrorHandler(FatalErrorCallback); mExternals = gcnew System::Collections::Generic::Dictionary(); + mMethods = gcnew System::Collections::Generic::Dictionary(); mTypeToConstructorMapping = gcnew System::Collections::Generic::Dictionary(); - mFunctions = gcnew System::Collections::Generic::List(); HandleScope scope(isolate); mContext = new Persistent(isolate, Context::New(isolate)); terminateRuns = false; @@ -175,18 +175,19 @@ JavascriptContext::~JavascriptContext() v8::Isolate::Scope isolate_scope(isolate); for each (WrappedJavascriptExternal wrapped in mExternals->Values) delete wrapped.Pointer; - for each (System::WeakReference^ f in mFunctions) { - JavascriptFunction ^function = safe_cast(f->Target); - if (function != nullptr) - delete function; + for each (WrappedMethod wrapped in mMethods->Values) + { + wrapped.Pointer->Reset(); + delete wrapped.Pointer; } for each (System::IntPtr p in mTypeToConstructorMapping->Values) { delete (void *)p; } delete mContext; + mContext = nullptr; delete mExternals; + delete mMethods; delete mTypeToConstructorMapping; - delete mFunctions; } if (isolate != NULL) isolate->Dispose(); @@ -525,16 +526,6 @@ JavascriptContext::GetObjectWrapperConstructorTemplate(System::Type ^type) //////////////////////////////////////////////////////////////////////////////////////////////////// -void -JavascriptContext::RegisterFunction(System::Object^ f) -{ - // Note that while we do store WeakReferences, we never clean up this hashtable, - // so it will just grow and grow. - mFunctions->Add(gcnew System::WeakReference(f)); -} - -//////////////////////////////////////////////////////////////////////////////////////////////////// - System::String^ JavascriptContext::V8Version::get() { return gcnew System::String(v8::V8::GetVersion()); diff --git a/Source/Noesis.Javascript/JavascriptContext.h b/Source/Noesis.Javascript/JavascriptContext.h index 553a6c8..3719bae 100644 --- a/Source/Noesis.Javascript/JavascriptContext.h +++ b/Source/Noesis.Javascript/JavascriptContext.h @@ -66,16 +66,16 @@ public enum class SetParameterOptions : int public value struct WrappedMethod { private: - System::IntPtr pointer; + System::IntPtr pointer; internal: - WrappedMethod(Persistent *value) - { - System::IntPtr value_pointer(value); + WrappedMethod(Persistent *value) + { + System::IntPtr value_pointer(value); pointer = value_pointer; - } + } - property Persistent *Pointer + property Persistent *Pointer { Persistent *get() { @@ -196,14 +196,20 @@ public ref class JavascriptContext: public System::IDisposable Local GetObjectWrapperConstructorTemplate(System::Type ^type); - void RegisterFunction(System::Object^ f); - static void FatalErrorCallbackMember(const char* location, const char* message); + inline bool IsDisposed() { return mContext == nullptr; } + //////////////////////////////////////////////////////////// // Data members //////////////////////////////////////////////////////////// - +internal: + // Stores every JavascriptExternal we create. This saves time if the same + // objects are recreated frequently, and stops us building up a huge + // collection of JavascriptExternal objects that won't be freed until + // the context is destroyed. + System::Collections::Generic::Dictionary^ mExternals; + System::Collections::Generic::Dictionary^ mMethods; protected: // By entering an isolate before using a context, we can have multiple // contexts used simultaneously in different threads. @@ -212,22 +218,12 @@ public ref class JavascriptContext: public System::IDisposable // v8 context required to be active for all v8 operations. Persistent* mContext; - // Stores every JavascriptExternal we create. This saves time if the same - // objects are recreated frequently, and stops us building up a huge - // collection of JavascriptExternal objects that won't be freed until - // the context is destroyed. - System::Collections::Generic::Dictionary ^mExternals; - // Maps types to their constructor function templates // The mapping will either be defined by the user calling `SetConstructor` or autogenerated if no // mapping was provided. // The `IntPtr` points to a `Persistent`. System::Collections::Generic::Dictionary ^mTypeToConstructorMapping; - // Stores every JavascriptFunction we create. Ensures we dispose of them - // all. - System::Collections::Generic::List ^mFunctions; - // See comment for TerminateExecution(). bool terminateRuns; diff --git a/Source/Noesis.Javascript/JavascriptExternal.cpp b/Source/Noesis.Javascript/JavascriptExternal.cpp index c01d5db..671e089 100644 --- a/Source/Noesis.Javascript/JavascriptExternal.cpp +++ b/Source/Noesis.Javascript/JavascriptExternal.cpp @@ -52,14 +52,38 @@ JavascriptExternal::JavascriptExternal(System::Object^ iObject) { mObjectHandle = System::Runtime::InteropServices::GCHandle::Alloc(iObject); mOptions = SetParameterOptions::None; - mMethods = gcnew System::Collections::Generic::Dictionary(); } //////////////////////////////////////////////////////////////////////////////////////////////////// JavascriptExternal::~JavascriptExternal() { - mObjectHandle.Free(); + mObjectHandle.Free(); + if (!mPersistent.IsEmpty()) { + mPersistent.ClearWeak(); + mPersistent.Reset(); + } + +} + +//////////////////////////////////////////////////////////////////////////////////////////////////// + +void GCCallback(const WeakCallbackInfo& data) +{ + auto context = JavascriptContext::GetCurrent(); + auto external = data.GetParameter(); + auto object = external->GetObject(); + if (context->mExternals->ContainsKey(object)) + context->mExternals->Remove(object); + delete external; +} + +void +JavascriptExternal::Wrap(Isolate* isolate, Local object) +{ + object->SetInternalField(0, External::New(isolate, this)); + mPersistent.Reset(isolate, object); + mPersistent.SetWeak(this, &GCCallback, WeakCallbackType::kParameter); } //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -75,40 +99,28 @@ JavascriptExternal::GetObject() Local JavascriptExternal::GetMethod(wstring iName) { - v8::Isolate *isolate = JavascriptContext::GetCurrentIsolate(); - System::Collections::Generic::Dictionary ^methods = mMethods; - System::String^ memberName = gcnew System::String(iName.c_str()); - WrappedMethod method; - if (methods->TryGetValue(memberName, method)) - return Local::New(isolate, *(method.Pointer)); - else - { - System::Object^ self = mObjectHandle.Target; - System::Type^ type = self->GetType(); - System::String^ memberName = gcnew System::String(iName.c_str()); - cli::array^ objectInfo = gcnew cli::array(2); - objectInfo->SetValue(self,0); - objectInfo->SetValue(memberName,1); - - // Verification if it a method - cli::array^ members = type->GetMember(memberName); - if (members->Length > 0 && members[0]->MemberType == MemberTypes::Method) - { - JavascriptContext^ context = JavascriptContext::GetCurrent(); - Local external = External::New(isolate, context->WrapObject(objectInfo)); - Local functionTemplate = FunctionTemplate::New(isolate, JavascriptInterop::Invoker, external); - Local function = functionTemplate->GetFunction(isolate->GetCurrentContext()).ToLocalChecked(); + auto context = JavascriptContext::GetCurrent(); + auto isolate = JavascriptContext::GetCurrentIsolate(); - Persistent *function_ptr = new Persistent(isolate, function); - WrappedMethod wrapped(function_ptr); - methods[memberName] = wrapped; + auto type = mObjectHandle.Target->GetType(); + auto memberName = gcnew System::String(iName.c_str()); + auto uniqueMethodName = type->AssemblyQualifiedName + L"." + memberName; - return function; - } - } + if (context->mMethods->ContainsKey(uniqueMethodName)) + return Local::New(isolate, *context->mMethods[uniqueMethodName].Pointer); + + // Verification if it a method + auto members = type->GetMember(memberName); + if (members->Length > 0 && members[0]->MemberType == MemberTypes::Method) + { + auto functionTemplate = FunctionTemplate::New(isolate, JavascriptInterop::Invoker, JavascriptInterop::ConvertToV8(memberName)); + auto function = functionTemplate->GetFunction(isolate->GetCurrentContext()).ToLocalChecked(); + context->mMethods[uniqueMethodName] = WrappedMethod(new Persistent(isolate, function)); + return function; + } // Wasn't an method - return Local(); + return Local(); } //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -333,37 +345,49 @@ JavascriptExternal::SetProperty(uint32_t iIndex, Local iValue) Local JavascriptExternal::GetIterator() { + auto context = JavascriptContext::GetCurrent(); + auto type = mObjectHandle.Target->GetType(); + auto uniqueMethodName = type->AssemblyQualifiedName + L".$$Iterator"; + auto isolate = JavascriptContext::GetCurrentIsolate(); - if (mIterator != nullptr) - return Local::New(isolate, *mIterator); + if (context->mMethods->ContainsKey(uniqueMethodName)) + return Local::New(isolate, *context->mMethods[uniqueMethodName].Pointer); - auto self = mObjectHandle.Target; - auto context = JavascriptContext::GetCurrent(); - auto external = External::New(isolate, context->WrapObject(self)); - auto functionTemplate = FunctionTemplate::New(isolate, JavascriptExternal::IteratorCallback, external); + auto functionTemplate = FunctionTemplate::New(isolate, JavascriptExternal::IteratorCallback); auto function = functionTemplate->GetFunction(isolate->GetCurrentContext()).ToLocalChecked(); - mIterator = std::make_unique>(isolate, function); + context->mMethods[uniqueMethodName] = WrappedMethod(new Persistent(isolate, function)); return function; } void JavascriptExternal::IteratorCallback(const v8::FunctionCallbackInfo& iArgs) { auto isolate = iArgs.GetIsolate(); - auto enumerable = (System::Collections::IEnumerable^) JavascriptInterop::UnwrapObject(Local::Cast(iArgs.Data())); - auto enumerator = enumerable->GetEnumerator(); - auto context = JavascriptContext::GetCurrent(); - auto external = External::New(isolate, context->WrapObject(enumerator)); auto iterator = ObjectTemplate::New(isolate); - auto functionTemplate = FunctionTemplate::New(isolate, JavascriptExternal::IteratorNextCallback, external); + iterator->SetInternalFieldCount(1); + auto functionTemplate = FunctionTemplate::New(isolate, JavascriptExternal::IteratorNextCallback); iterator->Set(String::NewFromUtf8(isolate, "next").ToLocalChecked(), functionTemplate); - iArgs.GetReturnValue().Set(iterator->NewInstance(isolate->GetCurrentContext()).ToLocalChecked()); + auto iteratorInstance = iterator->NewInstance(isolate->GetCurrentContext()).ToLocalChecked(); + + auto internalField = Local::Cast(iArgs.Holder()->GetInternalField(0)); + auto external = (JavascriptExternal*)internalField->Value(); + auto enumerable = (System::Collections::IEnumerable^)external->GetObject(); + auto enumerator = enumerable->GetEnumerator(); + + auto context = JavascriptContext::GetCurrent(); + auto enumeratorExternal = context->WrapObject(enumerator); + enumeratorExternal->Wrap(isolate, iteratorInstance); + + iArgs.GetReturnValue().Set(iteratorInstance); } void JavascriptExternal::IteratorNextCallback(const v8::FunctionCallbackInfo& iArgs) { auto isolate = iArgs.GetIsolate(); - auto enumerator = (System::Collections::IEnumerator^) JavascriptInterop::UnwrapObject(Local::Cast(iArgs.Data())); + + auto internalField = Local::Cast(iArgs.Holder()->GetInternalField(0)); + auto external = (JavascriptExternal*)internalField->Value(); + auto enumerator = (System::Collections::IEnumerator^) external->GetObject(); auto done = !enumerator->MoveNext(); auto resultTemplate = ObjectTemplate::New(isolate); @@ -378,4 +402,4 @@ void JavascriptExternal::IteratorNextCallback(const v8::FunctionCallbackInfo -#include #include #include "JavascriptContext.h" @@ -87,9 +86,13 @@ class JavascriptExternal Local GetIterator(); + void Wrap(Isolate* isolate, Local object); + //////////////////////////////////////////////////////////// // Data members //////////////////////////////////////////////////////////// +public: + Persistent mPersistent; private: // Handle to the .Net object being wrapped. It takes this @@ -98,10 +101,6 @@ class JavascriptExternal SetParameterOptions mOptions; - // Owned by JavascriptContext. - gcroot ^> mMethods; - - std::unique_ptr> mIterator; static void IteratorCallback(const v8::FunctionCallbackInfo& iArgs); static void IteratorNextCallback(const v8::FunctionCallbackInfo& iArgs); }; diff --git a/Source/Noesis.Javascript/JavascriptFunction.cpp b/Source/Noesis.Javascript/JavascriptFunction.cpp index e32d01d..6f112e1 100644 --- a/Source/Noesis.Javascript/JavascriptFunction.cpp +++ b/Source/Noesis.Javascript/JavascriptFunction.cpp @@ -20,18 +20,16 @@ JavascriptFunction::JavascriptFunction(v8::Local iFunction, Javascri throw gcnew System::ArgumentException("Must provide a JavascriptContext"); mFuncHandle = new Persistent(context->GetCurrentIsolate(), Local::Cast(iFunction)); - mContext = context; - - mContext->RegisterFunction(this); + mContextHandle = gcnew System::WeakReference(context); } JavascriptFunction::~JavascriptFunction() { if(mFuncHandle) { - if (mContext) + if (IsAlive()) { - JavascriptScope scope(mContext); + JavascriptScope scope(GetContext()); mFuncHandle->Reset(); } delete mFuncHandle; @@ -46,16 +44,17 @@ JavascriptFunction::!JavascriptFunction() System::Object^ JavascriptFunction::Call(... cli::array^ args) { - if (mFuncHandle == nullptr) + if (!IsAlive()) throw gcnew JavascriptException(L"This function's owning JavascriptContext has been disposed"); if (!args) throw gcnew System::ArgumentNullException("args"); - JavascriptScope scope(mContext); - v8::Isolate* isolate = mContext->GetCurrentIsolate(); + auto context = GetContext(); + JavascriptScope scope(context); + v8::Isolate* isolate = context->GetCurrentIsolate(); HandleScope handleScope(isolate); - Local global = mContext->GetGlobal(); + Local global = context->GetGlobal(); int argc = args->Length; Local *argv = new Local[argc]; @@ -81,13 +80,13 @@ bool JavascriptFunction::operator==(JavascriptFunction^ func1, JavascriptFunctio return false; if (func1_null && func2_null) return true; - if (func1->mFuncHandle == nullptr) + if (!func1->IsAlive()) throw gcnew JavascriptException(L"'func1's owning JavascriptContext has been disposed"); - if (func2->mFuncHandle == nullptr) + if (!func2->IsAlive()) throw gcnew JavascriptException(L"'func2's owning JavascriptContext has been disposed"); - Local jsFuncPtr1 = func1->mFuncHandle->Get(func1->mContext->GetCurrentIsolate()); - Local jsFuncPtr2 = func2->mFuncHandle->Get(func2->mContext->GetCurrentIsolate()); + Local jsFuncPtr1 = func1->mFuncHandle->Get(func1->GetContext()->GetCurrentIsolate()); + Local jsFuncPtr2 = func2->mFuncHandle->Get(func2->GetContext()->GetCurrentIsolate()); return jsFuncPtr1->Equals(JavascriptContext::GetCurrentIsolate()->GetCurrentContext(), jsFuncPtr2).ToChecked(); } @@ -99,22 +98,23 @@ bool JavascriptFunction::Equals(JavascriptFunction^ other) bool JavascriptFunction::Equals(Object^ other) { - if (mFuncHandle == nullptr) + if (!IsAlive()) throw gcnew JavascriptException(L"This function's owning JavascriptContext has been disposed"); JavascriptFunction^ otherFunc = dynamic_cast(other); - if (otherFunc != nullptr && otherFunc->mFuncHandle == nullptr) - throw gcnew JavascriptException(L"This function's owning JavascriptContext has been disposed"); + if (otherFunc != nullptr && !otherFunc->IsAlive()) + throw gcnew JavascriptException(L"The other function's owning JavascriptContext has been disposed"); return (otherFunc && this->Equals(otherFunc)); } System::String^ JavascriptFunction::ToString() { - if (mFuncHandle == nullptr) + if (!IsAlive()) throw gcnew JavascriptException(L"This function's owning JavascriptContext has been disposed"); - JavascriptScope scope(mContext); - auto isolate = mContext->GetCurrentIsolate(); + auto context = GetContext(); + JavascriptScope scope(context); + auto isolate = context->GetCurrentIsolate(); HandleScope handleScope(isolate); auto asString = mFuncHandle->Get(isolate)->ToString(isolate->GetCurrentContext()); return safe_cast(JavascriptInterop::ConvertFromV8(asString.ToLocalChecked())); diff --git a/Source/Noesis.Javascript/JavascriptFunction.h b/Source/Noesis.Javascript/JavascriptFunction.h index edd509a..472bd86 100644 --- a/Source/Noesis.Javascript/JavascriptFunction.h +++ b/Source/Noesis.Javascript/JavascriptFunction.h @@ -35,10 +35,12 @@ public ref class JavascriptFunction virtual bool Equals(Object^ other) override; virtual System::String^ ToString() override; - +internal: + v8::Persistent* mFuncHandle; private: - v8::Persistent* mFuncHandle; - JavascriptContext^ mContext; + System::WeakReference^ mContextHandle; + inline JavascriptContext^ GetContext() { return mContextHandle->IsAlive ? safe_cast(mContextHandle->Target) : nullptr; } + inline bool IsAlive() { auto context = GetContext(); return context != nullptr && !context->IsDisposed() && mFuncHandle != nullptr; } }; ////////////////////////////////////////////////////////////////////////// diff --git a/Source/Noesis.Javascript/JavascriptInterop.cpp b/Source/Noesis.Javascript/JavascriptInterop.cpp index 8fcd424..4f94c43 100644 --- a/Source/Noesis.Javascript/JavascriptInterop.cpp +++ b/Source/Noesis.Javascript/JavascriptInterop.cpp @@ -290,16 +290,18 @@ JavascriptInterop::WrapObject(System::Object^ iObject) { JavascriptContext^ context = JavascriptContext::GetCurrent(); - if (context != nullptr) - { - Local templ = context->GetObjectWrapperConstructorTemplate(iObject->GetType()); - v8::Isolate *isolate = JavascriptContext::GetCurrentIsolate(); + if (context != nullptr) + { + v8::Isolate *isolate = JavascriptContext::GetCurrentIsolate(); + JavascriptExternal *external = context->WrapObject(iObject); + + Local templ = context->GetObjectWrapperConstructorTemplate(iObject->GetType()); Local instanceTemplate = templ->InstanceTemplate(); - Local object = instanceTemplate->NewInstance(isolate->GetCurrentContext()).ToLocalChecked(); - object->SetInternalField(0, External::New(isolate, context->WrapObject(iObject))); + Local object = instanceTemplate->NewInstance(isolate->GetCurrentContext()).ToLocalChecked(); + external->Wrap(isolate, object); - return object; - } + return object; + } throw gcnew System::Exception("No context currently active."); } @@ -790,23 +792,20 @@ void JavascriptInterop::Invoker(const v8::FunctionCallbackInfo& iArgs) { v8::Isolate *isolate = JavascriptContext::GetCurrentIsolate(); - System::Object^ data = UnwrapObject(Local::Cast(iArgs.Data())); + auto internalField = Local::Cast(iArgs.Holder()->GetInternalField(0)); + auto external = (JavascriptExternal*)internalField->Value(); System::Reflection::MethodInfo^ bestMethod; cli::array^ suppliedArguments; cli::array^ bestMethodArguments; - cli::array^ objectInfo; int bestMethodMatchedArgs = -1; System::Object^ ret; - // get target and member's name - objectInfo = safe_cast^>(data); - System::Object^ self = objectInfo[0]; - // System::Object^ holder = UnwrapObject(iArgs.Holder()); + System::Object^ self = external->GetObject(); System::Type^ holderType = self->GetType(); // get members System::Type^ type = self->GetType(); - System::String^ memberName = (System::String^)objectInfo[1]; + System::String^ memberName = (System::String^) ConvertFromV8(iArgs.Data()); cli::array^ members = type->GetMember(memberName); if (members->Length > 0 && members[0]->MemberType == System::Reflection::MemberTypes::Method) diff --git a/Source/Noesis.Javascript/JavascriptInterop.h b/Source/Noesis.Javascript/JavascriptInterop.h index c7bfdde..8662ae4 100644 --- a/Source/Noesis.Javascript/JavascriptInterop.h +++ b/Source/Noesis.Javascript/JavascriptInterop.h @@ -114,8 +114,6 @@ class JavascriptInterop static System::Object^ ConvertArrayFromV8(Local iValue, ConvertedObjects &already_converted); - static Local WrapFunction(System::Object^ iObject, System::String^ iName); - static void Getter(Local iName, const PropertyCallbackInfo& iInfo); static void Setter(Local iName, Local iValue, const PropertyCallbackInfo& iInfo);