JNI: Fix reference leak in QJniArray

When the array type is QJniObject or a subclass, then we need to
explicitly release the local reference returned by GetObjectArrayElement
in the QJniArray::at implementation. Do this by constructing the
QJniObject via fromLocalRef, which does exactly that.
Amends 80d4d55e25.

Add a test case that stresses the local reference pool, and fix the old
test case (which operates on a QJniArray<jobject>) to also release the
local references.

Change-Id: Ie293b1db9f1b6825376bbf12338b22dfc3f8c6e9
Reviewed-by: Assam Boudjelthia <assam.boudjelthia@qt.io>
bb10
Volker Hilsheimer 2023-10-19 15:18:48 +02:00
parent fd48ce0b73
commit 72ad419116
2 changed files with 29 additions and 4 deletions

View File

@ -201,7 +201,11 @@ public:
{
JNIEnv *env = jniEnv();
if constexpr (std::is_convertible_v<jobject, T>) {
return T{env->GetObjectArrayElement(object<jobjectArray>(), i)};
jobject element = env->GetObjectArrayElement(object<jobjectArray>(), i);
if constexpr (std::is_base_of_v<QJniObject, T>)
return QJniObject::fromLocalRef(element);
else
return T{element};
} else {
T res = {};
if constexpr (std::is_same_v<T, jbyte>)

View File

@ -113,6 +113,7 @@ private slots:
void templateApiCheck();
void isClassAvailable();
void fromLocalRef();
void largeObjectArray();
void callback_data();
void callback();
@ -1534,9 +1535,10 @@ void tst_QJniObject::templateApiCheck()
const auto reverse = testClass.callMethod<jobject[]>("reverseObjectArray", newArray);
QVERIFY(reverse.isValid());
QCOMPARE(reverse.size(), 3);
QCOMPARE(QJniObject(reverse.at(0)).toString(), u"three"_s);
QCOMPARE(QJniObject(reverse.at(1)).toString(), u"two"_s);
QCOMPARE(QJniObject(reverse.at(2)).toString(), u"one"_s);
// QJniArray::at returns a jobject that's a local reference; make sure we don't free it twice
QCOMPARE(QJniObject::fromLocalRef(reverse.at(0)).toString(), u"three"_s);
QCOMPARE(QJniObject::fromLocalRef(reverse.at(1)).toString(), u"two"_s);
QCOMPARE(QJniObject::fromLocalRef(reverse.at(2)).toString(), u"one"_s);
}
// jbooleanArray ------------------------------------------------------------------------------
@ -1842,6 +1844,25 @@ void tst_QJniObject::fromLocalRef()
QJniObject o = QJniObject::fromLocalRef(env->FindClass("java/lang/String"));
}
void tst_QJniObject::largeObjectArray()
{
QJniArray<jobject> newArray(QList<QJniObject>{QJniObject::fromString(u"one"_s),
QJniObject::fromString(u"two"_s),
QJniObject::fromString(u"three"_s)});
QVERIFY(newArray.isValid());
const QJniArray<QJniObject> reverse = TestClass::callStaticMethod<jobject[]>(
"staticReverseObjectArray", newArray);
QVERIFY(reverse.isValid());
QCOMPARE(reverse.size(), 3);
// make sure we don't leak local references
for (int i = 0; i < 10000; ++i) {
QVERIFY(reverse.at(0).isValid());
QVERIFY(reverse.at(1).isValid());
QVERIFY(reverse.at(2).isValid());
}
}
static std::optional<TestClass> calledWithObject;
static int callbackWithObject(JNIEnv *, jobject, TestClass that)