From 285817f959fc8b16bcd610c032d29c5420c74a0d Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Mon, 20 Mar 2023 14:50:46 +0100 Subject: [PATCH] Fix creation of Bookmark. Bookmark wrapper can be created using two ways: - From a existing (cpp) bookmark (done internally by Library wrapper code) - As a totally new one (empty) the java code will have to setup (using `set*` methods). If the `Bookmark` constructor always create an empty new cpp bookmark, when we set the wrapper to point to the existing bookmark, we will have a leak of the new created bookmark. As we want to keep the "basic" constructor as the normal java api to create an empty bookmark, we need another (private) constructor to avoid the construction of an empty bookmark. The new constructor take a handle and directly set the `nativeHandle`. On `Library.getBookmarks` we cannot use the helper `BUILD_WRAPPER` and we must use "internal" function to use the `(J)V` constructor instead of the basic `()V`. --- lib/src/main/cpp/libkiwix/library.cpp | 8 +++++++- lib/src/main/java/org/kiwix/libkiwix/Bookmark.java | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/src/main/cpp/libkiwix/library.cpp b/lib/src/main/cpp/libkiwix/library.cpp index 25e9d99..cac6930 100644 --- a/lib/src/main/cpp/libkiwix/library.cpp +++ b/lib/src/main/cpp/libkiwix/library.cpp @@ -97,8 +97,14 @@ METHOD(jobjectArray, getBookmarks, jboolean onlyValidBookmarks) { auto bookmarks = THIS->getBookmarks(TO_C(onlyValidBookmarks)); jobjectArray retArray = createArray(env, bookmarks.size(), "org/kiwix/libkiwix/Bookmark"); size_t index = 0; + jclass wrapperClass = env->FindClass("org/kiwix/libkiwix/Bookmark"); + jmethodID initMethod = env->GetMethodID(wrapperClass, "", "(J)V"); + for (auto bookmark: bookmarks) { - auto wrapper = BUILD_WRAPPER("org/kiwix/libkiwix/Bookmark", bookmark); + // This double new is necessary as we need to allocate the bookmark itself (as a shared_ptr) on the heap but + // we also want the shared_ptr to be stored in the head as we want to have a ptr (cast as long) to it. + shared_ptr* handle = new shared_ptr(new kiwix::Bookmark(std::move(bookmark))); + jobject wrapper = env->NewObject(wrapperClass, initMethod, reinterpret_cast(handle)); env->SetObjectArrayElement(retArray, index++, wrapper); } return retArray; diff --git a/lib/src/main/java/org/kiwix/libkiwix/Bookmark.java b/lib/src/main/java/org/kiwix/libkiwix/Bookmark.java index a6c1d4c..936ab57 100644 --- a/lib/src/main/java/org/kiwix/libkiwix/Bookmark.java +++ b/lib/src/main/java/org/kiwix/libkiwix/Bookmark.java @@ -25,6 +25,10 @@ public class Bookmark setNativeBookmark(); } + private Bookmark(long handle) { + nativeHandle = handle; + } + public native void setBookId(String bookId); public native void setBookTitle(String bookTitle); public native void setUrl(String url);