Skip to content

Commit 642c733

Browse files
joefernerjoeferner
committed
added local frames to javaObject method calls to try and
prevent local leaks
1 parent 07fd0db commit 642c733

2 files changed

Lines changed: 56 additions & 11 deletions

File tree

‎src/javaObject.cpp‎

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@
1919

2020
/*static*/ v8::Local<v8::Object> JavaObject::New(Java *java, jobject obj) {
2121
v8::HandleScope scope;
22+
23+
JNIEnv *env = java->getJavaEnv();
24+
PUSH_LOCAL_JAVA_FRAME();
25+
2226
v8::Local<v8::Function> ctor = s_ct->GetFunction();
2327
v8::Local<v8::Object> javaObjectObj = ctor->NewInstance();
2428
JavaObject *self = new JavaObject(java, obj);
2529
self->Wrap(javaObjectObj);
2630

27-
JNIEnv *env = self->m_java->getJavaEnv();
28-
2931
std::list<jobject> methods;
3032
javaReflectionGetMethods(env, self->m_class, &methods);
3133
jclass methodClazz = env->FindClass("java/lang/reflect/Method");
@@ -61,13 +63,16 @@
6163
env->DeleteLocalRef(*it);
6264
}
6365

66+
POP_LOCAL_JAVA_FRAME();
67+
6468
return scope.Close(javaObjectObj);
6569
}
6670

6771
JavaObject::JavaObject(Java *java, jobject obj) {
6872
m_java = java;
69-
m_obj = m_java->getJavaEnv()->NewGlobalRef(obj);
70-
m_class = m_java->getJavaEnv()->GetObjectClass(obj);
73+
JNIEnv *env = m_java->getJavaEnv();
74+
m_obj = env->NewGlobalRef(obj);
75+
m_class = (jclass)env->NewGlobalRef(env->GetObjectClass(obj));
7176
}
7277

7378
JavaObject::~JavaObject() {
@@ -82,14 +87,17 @@ JavaObject::~JavaObject() {
8287
}
8388
}
8489

85-
m_java->getJavaEnv()->DeleteGlobalRef(m_obj);
90+
env->DeleteGlobalRef(m_obj);
91+
env->DeleteGlobalRef(m_class);
8692
}
8793

8894
/*static*/ v8::Handle<v8::Value> JavaObject::methodCall(const v8::Arguments& args) {
8995
v8::HandleScope scope;
9096
JavaObject* self = node::ObjectWrap::Unwrap<JavaObject>(args.This());
9197
JNIEnv *env = self->m_java->getJavaEnv();
9298

99+
PUSH_LOCAL_JAVA_FRAME();
100+
93101
v8::String::AsciiValue methodName(args.Data());
94102
std::string methodNameStr = *methodName;
95103

@@ -100,6 +108,7 @@ JavaObject::~JavaObject() {
100108
ARGS_BACK_CALLBACK();
101109

102110
if(!callbackProvided && methodNameStr == "toString") {
111+
POP_LOCAL_JAVA_FRAME();
103112
return methodCallSync(args);
104113
}
105114

@@ -108,6 +117,7 @@ JavaObject::~JavaObject() {
108117
jobject method = javaFindMethod(env, self->m_class, methodNameStr, methodArgs);
109118
if(method == NULL) {
110119
EXCEPTION_CALL_CALLBACK("Could not find method " << methodNameStr);
120+
POP_LOCAL_JAVA_FRAME();
111121
return v8::Undefined();
112122
}
113123

@@ -117,6 +127,7 @@ JavaObject::~JavaObject() {
117127

118128
env->DeleteLocalRef(methodArgs);
119129
env->DeleteLocalRef(method);
130+
POP_LOCAL_JAVA_FRAME();
120131

121132
END_CALLBACK_FUNCTION("\"Method '" << methodNameStr << "' called without a callback did you mean to use the Sync version?\"");
122133
}
@@ -126,6 +137,8 @@ JavaObject::~JavaObject() {
126137
JavaObject* self = node::ObjectWrap::Unwrap<JavaObject>(args.This());
127138
JNIEnv *env = self->m_java->getJavaEnv();
128139

140+
PUSH_LOCAL_JAVA_FRAME();
141+
129142
v8::String::AsciiValue methodName(args.Data());
130143
std::string methodNameStr = *methodName;
131144

@@ -138,29 +151,38 @@ JavaObject::~JavaObject() {
138151
if(method == NULL) {
139152
std::ostringstream errStr;
140153
errStr << "Could not find method " << methodNameStr;
141-
return ThrowException(javaExceptionToV8(env, errStr.str()));
154+
v8::Handle<v8::Value> ex = javaExceptionToV8(env, errStr.str());
155+
POP_LOCAL_JAVA_FRAME();
156+
return ThrowException(ex);
142157
}
143158

144159
// run
145160
v8::Handle<v8::Value> callback = v8::Object::New();
146161
InstanceMethodCallBaton* baton = new InstanceMethodCallBaton(self->m_java, self, method, methodArgs, callback);
147162
v8::Handle<v8::Value> result = baton->runSync();
148163
delete baton;
149-
return scope.Close(result);;
164+
165+
POP_LOCAL_JAVA_FRAME();
166+
167+
return scope.Close(result);
150168
}
151169

152170
/*static*/ v8::Handle<v8::Value> JavaObject::fieldGetter(v8::Local<v8::String> property, const v8::AccessorInfo& info) {
153171
v8::HandleScope scope;
154172
JavaObject* self = node::ObjectWrap::Unwrap<JavaObject>(info.This());
155173
JNIEnv *env = self->m_java->getJavaEnv();
156174

175+
PUSH_LOCAL_JAVA_FRAME();
176+
157177
v8::String::AsciiValue propertyCStr(property);
158178
std::string propertyStr = *propertyCStr;
159179
jobject field = javaFindField(env, self->m_class, propertyStr);
160180
if(field == NULL) {
161181
std::ostringstream errStr;
162182
errStr << "Could not find field " << propertyStr;
163-
return ThrowException(javaExceptionToV8(env, errStr.str()));
183+
v8::Handle<v8::Value> ex = javaExceptionToV8(env, errStr.str());
184+
POP_LOCAL_JAVA_FRAME();
185+
return ThrowException(ex);
164186
}
165187

166188
jclass fieldClazz = env->FindClass("java/lang/reflect/Field");
@@ -171,13 +193,16 @@ JavaObject::~JavaObject() {
171193
if(env->ExceptionOccurred()) {
172194
std::ostringstream errStr;
173195
errStr << "Could not get field " << propertyStr;
174-
return ThrowException(javaExceptionToV8(env, errStr.str()));
196+
v8::Handle<v8::Value> ex = javaExceptionToV8(env, errStr.str());
197+
POP_LOCAL_JAVA_FRAME();
198+
return ThrowException(ex);
175199
}
176200

177201
v8::Handle<v8::Value> result = javaToV8(self->m_java, env, val);
178202

179203
env->DeleteLocalRef(field);
180204
env->DeleteLocalRef(val);
205+
POP_LOCAL_JAVA_FRAME();
181206

182207
return scope.Close(result);
183208
}
@@ -187,6 +212,8 @@ JavaObject::~JavaObject() {
187212
JavaObject* self = node::ObjectWrap::Unwrap<JavaObject>(info.This());
188213
JNIEnv *env = self->m_java->getJavaEnv();
189214

215+
PUSH_LOCAL_JAVA_FRAME();
216+
190217
jobject newValue = v8ToJava(env, value);
191218

192219
v8::String::AsciiValue propertyCStr(property);
@@ -195,7 +222,9 @@ JavaObject::~JavaObject() {
195222
if(field == NULL) {
196223
std::ostringstream errStr;
197224
errStr << "Could not find field " << propertyStr;
198-
ThrowException(javaExceptionToV8(env, errStr.str()));
225+
v8::Handle<v8::Value> ex = javaExceptionToV8(env, errStr.str());
226+
POP_LOCAL_JAVA_FRAME();
227+
ThrowException(ex);
199228
return;
200229
}
201230

@@ -209,7 +238,11 @@ JavaObject::~JavaObject() {
209238
if(env->ExceptionOccurred()) {
210239
std::ostringstream errStr;
211240
errStr << "Could not set field " << propertyStr;
212-
ThrowException(javaExceptionToV8(env, errStr.str()));
241+
v8::Handle<v8::Value> ex = javaExceptionToV8(env, errStr.str());
242+
POP_LOCAL_JAVA_FRAME();
243+
ThrowException(ex);
213244
return;
214245
}
246+
247+
POP_LOCAL_JAVA_FRAME();
215248
}

‎src/utils.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ struct DynamicProxyData {
3737
unsigned int markerEnd;
3838
};
3939

40+
#define LOCAL_FRAME_SIZE 500
41+
4042
#define DYNAMIC_PROXY_DATA_MARKER_START 0x12345678
4143
#define DYNAMIC_PROXY_DATA_MARKER_END 0x87654321
4244

@@ -117,4 +119,14 @@ jobject javaFindConstructor(JNIEnv *env, jclass clazz, jobjectArray methodArgs);
117119
return scope.Close(v8::String::New(str.str().c_str())); \
118120
}
119121

122+
#define PUSH_LOCAL_JAVA_FRAME() \
123+
env->PushLocalFrame(LOCAL_FRAME_SIZE);
124+
125+
#define POP_LOCAL_JAVA_FRAME() \
126+
env->PopLocalFrame(NULL);
127+
128+
#define POP_LOCAL_JAVA_FRAME_AND_RETURN(r) \
129+
env->PopLocalFrame(NULL); \
130+
return r;
131+
120132
#endif

0 commit comments

Comments
 (0)