Skip to content

Commit

Permalink
🐞 fix: #423 Fix memory leaks in function callback in edge cases
Browse files Browse the repository at this point in the history
  • Loading branch information
@caoccao
caoccao committed Dec 5, 2024
1 parent 5c7e7da commit 0672a1c
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 17 deletions.
24 changes: 16 additions & 8 deletions cpp/jni/javet_callbacks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,11 @@ namespace Javet {
else {
FETCH_JNI_ENV(GlobalJavaVM);
auto externalV8Runtime = v8Runtime->externalV8Runtime;
jobject mIV8Module = jniEnv->CallObjectMethod(
externalV8Runtime, jmethodIDV8RuntimeReceiveGCEpilogueCallback, (jint)v8GCType, (jint)v8GCCallbackFlags);
jniEnv->CallVoidMethod(
externalV8Runtime,
jmethodIDV8RuntimeReceiveGCEpilogueCallback,
(jint)v8GCType,
(jint)v8GCCallbackFlags);
}
}
}
Expand All @@ -156,8 +159,11 @@ namespace Javet {
else {
FETCH_JNI_ENV(GlobalJavaVM);
auto externalV8Runtime = v8Runtime->externalV8Runtime;
jobject mIV8Module = jniEnv->CallObjectMethod(
externalV8Runtime, jmethodIDV8RuntimeReceiveGCPrologueCallback, (jint)v8GCType, (jint)v8GCCallbackFlags);
jniEnv->CallVoidMethod(
externalV8Runtime,
jmethodIDV8RuntimeReceiveGCPrologueCallback,
(jint)v8GCType,
(jint)v8GCCallbackFlags);
}
}
}
Expand Down Expand Up @@ -224,9 +230,11 @@ namespace Javet {
auto v8PersistentModule = TO_V8_PERSISTENT_MODULE_POINTER(mHandle);
LOG_DEBUG("JavetModuleResolveCallback: module '" << moduleNamePointer.get() << "' found");
resolvedV8MaybeLocalModule = v8PersistentModule->Get(v8Context->GetIsolate());
DELETE_LOCAL_REF(jniEnv, mIV8Module);
}
if (mReferrerV8Module != nullptr) {
jniEnv->CallVoidMethod(mReferrerV8Module, jmethodIDIV8ValueReferenceClose, true);
DELETE_LOCAL_REF(jniEnv, mReferrerV8Module);
}
}
}
Expand Down Expand Up @@ -389,7 +397,7 @@ namespace Javet {
if (jniEnv->ExceptionCheck()) {
if (mResult != nullptr) {
jniEnv->CallStaticVoidMethod(jclassJavetResourceUtils, jmethodIDJavetResourceUtilsSafeClose, mResult);
jniEnv->DeleteLocalRef(mResult);
DELETE_LOCAL_REF(jniEnv, mResult);
}
Javet::Exceptions::ThrowV8Exception(jniEnv, v8Context, "Uncaught JavaError in function callback");
}
Expand All @@ -404,7 +412,7 @@ namespace Javet {
}
if (mResult != nullptr) {
jniEnv->CallStaticVoidMethod(jclassJavetResourceUtils, jmethodIDJavetResourceUtilsSafeClose, mResult);
jniEnv->DeleteLocalRef(mResult);
DELETE_LOCAL_REF(jniEnv, mResult);
}
}
}
Expand Down Expand Up @@ -457,7 +465,7 @@ namespace Javet {
}
if (mResult != nullptr) {
jniEnv->CallStaticVoidMethod(jclassJavetResourceUtils, jmethodIDJavetResourceUtilsSafeClose, mResult);
jniEnv->DeleteLocalRef(mResult);
DELETE_LOCAL_REF(jniEnv, mResult);
if (jniEnv->ExceptionCheck()) {
Javet::Exceptions::ThrowV8Exception(jniEnv, v8Context, "Uncaught JavaError in property getter callback");
}
Expand Down Expand Up @@ -511,7 +519,7 @@ namespace Javet {
}
if (mResult != nullptr) {
jniEnv->CallStaticVoidMethod(jclassJavetResourceUtils, jmethodIDJavetResourceUtilsSafeClose, mResult);
jniEnv->DeleteLocalRef(mResult);
DELETE_LOCAL_REF(jniEnv, mResult);
if (jniEnv->ExceptionCheck()) {
Javet::Exceptions::ThrowV8Exception(jniEnv, v8Context, "Uncaught JavaError in property setter callback");
}
Expand Down
31 changes: 23 additions & 8 deletions cpp/jni/javet_converter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -720,21 +720,31 @@ namespace Javet {
}
if (v8Value->IsArrayBuffer()) {
auto v8ArrayBuffer = v8Value.As<v8::ArrayBuffer>();
return jniEnv->NewObject(
jobject directByteBuffer = jniEnv->NewDirectByteBuffer(
v8ArrayBuffer->GetBackingStore()->Data(),
v8ArrayBuffer->ByteLength());
jobject v8ValueArrayBuffer = jniEnv->NewObject(
jclassV8ValueArrayBuffer,
jmethodIDV8ValueArrayBufferConstructor,
v8Runtime->externalV8Runtime,
ToV8PersistentReference(v8Context, v8Value),
jniEnv->NewDirectByteBuffer(v8ArrayBuffer->GetBackingStore()->Data(), v8ArrayBuffer->ByteLength()));
directByteBuffer);
DELETE_LOCAL_REF(jniEnv, directByteBuffer);
return v8ValueArrayBuffer;
}
if (v8Value->IsSharedArrayBuffer()) {
auto v8SharedArrayBuffer = v8Value.As<v8::SharedArrayBuffer>();
return jniEnv->NewObject(
jobject directByteBuffer = jniEnv->NewDirectByteBuffer(
v8SharedArrayBuffer->GetBackingStore()->Data(),
v8SharedArrayBuffer->ByteLength());
jobject v8ValueSharedArrayBuffer = jniEnv->NewObject(
jclassV8ValueSharedArrayBuffer,
jmethodIDV8ValueSharedArrayBufferConstructor,
v8Runtime->externalV8Runtime,
ToV8PersistentReference(v8Context, v8Value),
jniEnv->NewDirectByteBuffer(v8SharedArrayBuffer->GetBackingStore()->Data(), v8SharedArrayBuffer->ByteLength()));
directByteBuffer);
DELETE_LOCAL_REF(jniEnv, directByteBuffer);
return v8ValueSharedArrayBuffer;
}
if (v8Value->IsArrayBufferView()) {
/*
Expand Down Expand Up @@ -939,12 +949,14 @@ namespace Javet {
v8LocalBigInt->ToWordsArray(&signBit, &wordCount, reinterpret_cast<uint64_t*>(mLongArrayPointer));
jniEnv->ReleaseLongArrayElements(mLongArray, mLongArrayPointer, 0);
jint signum = signBit == 0 ? 1 : -1;
return jniEnv->NewObject(
jobject v8ValueBigInteger = jniEnv->NewObject(
jclassV8ValueBigInteger,
jmethodIDV8ValueBigIntegerConstructor,
v8Runtime->externalV8Runtime,
signum,
mLongArray);
DELETE_LOCAL_REF(jniEnv, mLongArray);
return v8ValueBigInteger;
}
}
// Something is wrong. It defaults to toString().
Expand All @@ -968,7 +980,9 @@ namespace Javet {
// TODO: Memory leak might take place.
v8ValueArray = jniEnv->NewObjectArray(argLength, jclassV8Value, nullptr);
for (int i = 0; i < argLength; ++i) {
jniEnv->SetObjectArrayElement(v8ValueArray, i, ToExternalV8Value(jniEnv, v8Runtime, v8Context, args[i]));
jobject v8Value = ToExternalV8Value(jniEnv, v8Runtime, v8Context, args[i]);
jniEnv->SetObjectArrayElement(v8ValueArray, i, v8Value);
DELETE_LOCAL_REF(jniEnv, v8Value);
}
}
return v8ValueArray;
Expand Down Expand Up @@ -1016,8 +1030,9 @@ namespace Javet {
else {
v8LocalValue = v8MaybeLocalValue.ToLocalChecked();
}
jniEnv->SetObjectArrayElement(
v8Values, i, ToExternalV8Value(jniEnv, v8Runtime, v8Context, v8LocalValue));
jobject v8Value = ToExternalV8Value(jniEnv, v8Runtime, v8Context, v8LocalValue);
jniEnv->SetObjectArrayElement(v8Values, i, v8Value);
DELETE_LOCAL_REF(jniEnv, v8Value);
}
return actualLength;
}
Expand Down
2 changes: 1 addition & 1 deletion cpp/jni/javet_converter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ namespace Javet {
jmethodIDV8ValuePrimitiveConstructor,
v8Runtime->externalV8Runtime,
mStringValue);
jniEnv->DeleteLocalRef(mStringValue);
DELETE_LOCAL_REF(jniEnv, mStringValue);
return mV8ValuePrimitive;
}

Expand Down
1 change: 1 addition & 0 deletions docs/release_notes/release_notes_4_0.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Release Notes 4.0.x - 4.1.x

* Upgraded Visual Studio 2022 to `v17.12.0 <https://learn.microsoft.com/en-us/visualstudio/releases/2022/release-notes-v17.12>`_
* Added ``getAbsoluteResourceName()`` to ``IV8ModuleResolver``
* Fixed memory leaks in function callback in edge cases

4.1.0
-----
Expand Down

0 comments on commit 0672a1c

Please sign in to comment.