Skip to content

Commit 0350ee4

Browse files
committed
[WIP] Use a common class Wrapper and IterWrapper to wrapp native resource.
Native resource (pointer to shared_ptr) is stored as a long in the `Wrapper.Resource` class. As Wrapper implements AutoCloseable and we register the (java) resource to be cleaned at object destruction, we now properly delete the native resource. This also adding new macro to avoid writting the class name and so reduce potential typos.
1 parent 06638d4 commit 0350ee4

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+674
-487
lines changed

lib/build.gradle

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ ext {
2626

2727
apply from: 'publish.gradle'
2828
android {
29-
compileSdk 32
29+
compileSdk 33
3030

3131
defaultConfig {
3232

33-
minSdk 21
33+
minSdk 33
3434
targetSdk 32
3535
versionCode 1
3636
versionName "1.0"
@@ -285,5 +285,5 @@ task checkCurrentJavaVersion() {
285285

286286
task generateHeaderFilesFromJavaWrapper(type: Exec) {
287287
workingDir "${projectDir}/src/main/java/org/kiwix/"
288-
commandLine 'bash', '-c', "javac -h ${buildDir}/include/javah_generated/ -d ${buildDir}/libzim/ libzim/*.java libkiwix/*.java"
288+
commandLine 'bash', '-c', "javac -h ${buildDir}/include/javah_generated/ -d ${buildDir}/libzim/ *.java libzim/*.java libkiwix/*.java"
289289
}

lib/src/main/cpp/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ add_library(
1010
zim_wrapper
1111

1212
SHARED
13+
wrapper.cpp
1314
libzim/archive.cpp
1415
libzim/entry.cpp
1516
libzim/entry_iterator.cpp
@@ -45,6 +46,7 @@ add_library(
4546
kiwix_wrapper
4647

4748
SHARED
49+
wrapper.cpp
4850
libkiwix/book.cpp
4951
libkiwix/filter.cpp
5052
libkiwix/kiwixicu.cpp

lib/src/main/cpp/libkiwix/book.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,11 @@
2929
#define TYPENAME libkiwix_Book
3030
#include <macros.h>
3131

32-
METHOD0(void, allocate)
32+
METHOD0(jobject, getNativeBook)
3333
{
34-
SET_PTR(std::make_shared<NATIVE_TYPE>());
34+
return NEW_RESOURCE(std::make_shared<NATIVE_TYPE>());
3535
}
3636

37-
DISPOSE
38-
3937
METHOD(void, update__Lorg_kiwix_libkiwix_Book_2, jobject otherBook)
4038
{
4139
THIS->update(*getPtr<kiwix::Book>(env, otherBook));
@@ -93,12 +91,12 @@ METHOD0(jobjectArray, getIllustrations) {
9391
jobjectArray retArray = createArray(env, illustrations.size(), "org/kiwix/libkiwix/Illustration");
9492
size_t index = 0;
9593
for (auto illu: illustrations) {
96-
auto wrapper = BUILD_WRAPPER("org/kiwix/libkiwx/Illustration", illu);
94+
auto wrapper = BUILD_WRAPPER(illu);
9795
env->SetObjectArrayElement(retArray, index++, wrapper);
9896
}
9997
return retArray;
10098
}
10199

102100
METHOD(jobject, getIllustration, jint size) {
103-
return BUILD_WRAPPER("org/kiwix/libkiwix/Illustration", THIS->getIllustration(TO_C(size)));
101+
return BUILD_WRAPPER(THIS->getIllustration(TO_C(size)));
104102
}

lib/src/main/cpp/libkiwix/filter.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,11 @@
2828
#define TYPENAME libkiwix_Filter
2929
#include "macros.h"
3030

31-
32-
3331
/* Kiwix Reader JNI functions */
34-
METHOD0(void, allocate) {
35-
SET_PTR(std::make_shared<NATIVE_TYPE>());
32+
METHOD0(jobject, getNativeFilter) {
33+
return NEW_RESOURCE(std::make_shared<NATIVE_TYPE>());
3634
}
3735

38-
DISPOSE
39-
4036
#define FORWARD(name, args_type) \
4137
METHOD(jobject, name, args_type value) { \
4238
THIS->name(jni2c(value, env)); \

lib/src/main/cpp/libkiwix/kiwixserver.cpp

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,14 @@
3232

3333

3434
/* Kiwix Reader JNI functions */
35-
METHOD(void, setNativeServer, jobject jLibrary)
35+
METHOD(jobject, buildNativeServer, jobject jLibrary)
3636
{
3737
LOG("Attempting to create server");
38-
try {
39-
auto library = getPtr<kiwix::Library>(env, jLibrary);
40-
SET_PTR(std::make_shared<NATIVE_TYPE>(library.get()));
41-
} catch (std::exception& e) {
42-
LOG("Error creating the server");
43-
LOG("%s", e.what());
44-
}
38+
auto library = getPtr<kiwix::Library>(env, jLibrary);
39+
return NEW_RESOURCE(std::make_shared<NATIVE_TYPE>(library.get()));
4540
}
4641

4742

48-
DISPOSE
49-
5043
/* Kiwix library functions */
5144
METHOD(void, setRoot, jstring root)
5245
{

lib/src/main/cpp/libkiwix/library.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,11 @@
2929
#include "macros.h"
3030

3131
/* Kiwix Reader JNI functions */
32-
METHOD0(void, setNativeHandler)
32+
METHOD0(jobject, buildNativeLibrary)
3333
{
34-
SET_PTR(std::make_shared<NATIVE_TYPE>());
34+
return NEW_RESOURCE(std::make_shared<NATIVE_TYPE>());
3535
}
3636

37-
DISPOSE
3837

3938
/* Kiwix library functions */
4039
METHOD(jboolean, addBook, jobject book)
@@ -50,11 +49,11 @@ METHOD(jboolean, addBook, jobject book)
5049
}
5150

5251
METHOD(jobject, getBookById, jstring id) {
53-
return BUILD_WRAPPER("org/kiwix/libkiwix/Book", THIS->getBookById(TO_C(id)));
52+
return BUILD_WRAPPER(THIS->getBookById(TO_C(id)));
5453
}
5554

5655
METHOD(jobject, getArchiveById, jstring id) {
57-
return BUILD_WRAPPER("org/kiwix/libzim/Archive", THIS->getArchiveById(TO_C(id)));
56+
return BUILD_WRAPPER(THIS->getArchiveById(TO_C(id)));
5857
}
5958

6059
METHOD(jboolean, removeBookById, jstring id) {

lib/src/main/cpp/libkiwix/manager.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,12 @@
2828
#define TYPENAME libkiwix_Manager
2929
#include <macros.h>
3030

31-
METHOD(void, allocate, jobject libraryObj)
31+
METHOD(jobject, buildNativeManager, jobject libraryObj)
3232
{
3333
auto lib = getPtr<kiwix::Library>(env, libraryObj);
34-
SET_PTR(std::make_shared<NATIVE_TYPE>(lib.get()));
34+
return NEW_RESOURCE(std::make_shared<NATIVE_TYPE>(lib.get()));
3535
}
3636

37-
DISPOSE
38-
3937
/* Kiwix manager functions */
4038
METHOD(jboolean, readFile, jstring path)
4139
{

lib/src/main/cpp/libzim/archive.cpp

Lines changed: 51 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,13 @@
3636
#include <macros.h>
3737

3838
/* Kiwix Reader JNI functions */
39-
METHOD(void, setNativeArchive, jstring filename)
39+
METHOD(jobject, buildNativeArchive, jstring filename)
4040
{
4141
std::string cPath = TO_C(filename);
4242

4343
LOG("Attempting to create reader with: %s", cPath.c_str());
44-
try {
45-
auto archive = std::make_shared<zim::Archive>(cPath);
46-
SET_PTR(archive);
47-
} catch (std::exception& e) {
48-
LOG("Error opening ZIM file");
49-
LOG("%s", e.what());
50-
}
44+
auto archive = std::make_shared<NATIVE_TYPE>(cPath);
45+
return NEW_RESOURCE(archive);
5146
}
5247

5348
namespace
@@ -70,7 +65,7 @@ int jni2fd(const jobject& fdObj, JNIEnv* env)
7065

7166
} // unnamed namespace
7267

73-
JNIEXPORT void JNICALL Java_org_kiwix_libzim_Archive_setNativeArchiveByFD(
68+
JNIEXPORT jobject JNICALL Java_org_kiwix_libzim_Archive_buildNativeArchiveByFD(
7469
JNIEnv* env, jobject thisObj, jobject fdObj)
7570
{
7671
#ifndef _WIN32
@@ -79,18 +74,16 @@ JNIEXPORT void JNICALL Java_org_kiwix_libzim_Archive_setNativeArchiveByFD(
7974
LOG("Attempting to create reader with fd: %d", fd);
8075
try {
8176
auto archive = std::make_shared<zim::Archive>(fd);
82-
SET_PTR(archive);
83-
} catch (std::exception& e) {
84-
LOG("Error opening ZIM file");
85-
LOG("%s", e.what());
86-
}
77+
return NEW_RESOURCE(archive);
78+
} CATCH("Error opening ZIM file")
8779
#else
8880
jclass exception = env->FindClass("java/lang/UnsupportedOperationException");
8981
env->ThrowNew(exception, "org.kiwix.libzim.Archive.setNativeArchiveByFD() is not supported under Windows");
9082
#endif
83+
return nullptr;
9184
}
9285

93-
JNIEXPORT void JNICALL Java_org_kiwix_libzim_Archive_setNativeArchiveEmbedded(
86+
JNIEXPORT jobject JNICALL Java_org_kiwix_libzim_Archive_buildNativeArchiveEmbedded(
9487
JNIEnv* env, jobject thisObj, jobject fdObj, jlong offset, jlong size)
9588
{
9689
#ifndef _WIN32
@@ -99,19 +92,15 @@ JNIEXPORT void JNICALL Java_org_kiwix_libzim_Archive_setNativeArchiveEmbedded(
9992
LOG("Attempting to create reader with fd: %d", fd);
10093
try {
10194
auto archive = std::make_shared<zim::Archive>(fd, offset, size);
102-
SET_PTR(archive);
103-
} catch (std::exception& e) {
104-
LOG("Error opening ZIM file");
105-
LOG("%s", e.what());
106-
}
95+
return NEW_RESOURCE(archive);
96+
} CATCH("Error opening ZIM file")
10797
#else
10898
jclass exception = env->FindClass("java/lang/UnsupportedOperationException");
10999
env->ThrowNew(exception, "org.kiwix.libzim.Archive.setNativeArchiveEmbedded() is not supported under Windows");
110100
#endif
101+
return nullptr;
111102
}
112103

113-
DISPOSE
114-
115104
GETTER(jstring, getFilename)
116105
GETTER(jlong, getFilesize)
117106
GETTER(jint, getAllEntryCount)
@@ -128,13 +117,13 @@ METHOD(jstring, getMetadata, jstring name) {
128117
}
129118

130119
METHOD(jobject, getMetadataItem, jstring name) {
131-
return BUILD_WRAPPER("org/kiwix/libzim/Item", THIS->getMetadataItem(TO_C(name)));
120+
return BUILD_WRAPPER(THIS->getMetadataItem(TO_C(name)));
132121
}
133122

134123
GETTER(jobjectArray, getMetadataKeys)
135124

136125
METHOD(jobject, getIllustrationItem, jint size) {
137-
return BUILD_WRAPPER("org/kiwix/libzim/Item", THIS->getIllustrationItem(TO_C(size)));
126+
return BUILD_WRAPPER(THIS->getIllustrationItem(TO_C(size)));
138127
}
139128

140129
METHOD(jboolean, hasIllustration, jint size) {
@@ -144,41 +133,41 @@ METHOD(jboolean, hasIllustration, jint size) {
144133
GETTER(jlongArray, getIllustrationSizes)
145134

146135
METHOD(jobject, getEntryByPath__Ljava_lang_String_2, jstring path) {
147-
return BUILD_WRAPPER("org/kiwix/libzim/Entry", THIS->getEntryByPath(TO_C(path)));
136+
return BUILD_WRAPPER(THIS->getEntryByPath(TO_C(path)));
148137
}
149138

150139
METHOD(jobject, getEntryByPath__I, jint index) {
151-
return BUILD_WRAPPER("org/kiwix/libzim/Entry", THIS->getEntryByPath(TO_C(index)));
140+
return BUILD_WRAPPER(THIS->getEntryByPath(TO_C(index)));
152141
}
153142

154143
METHOD(jboolean, hasEntryByPath, jstring path) {
155144
return TO_JNI(THIS->hasEntryByPath(TO_C(path)));
156145
}
157146

158147
METHOD(jobject, getEntryByTitle__Ljava_lang_String_2, jstring title) {
159-
return BUILD_WRAPPER("org/kiwix/libzim/Entry", THIS->getEntryByTitle(TO_C(title)));
148+
return BUILD_WRAPPER(THIS->getEntryByTitle(TO_C(title)));
160149
}
161150

162151
METHOD(jobject, getEntryByTitle__I, jint index) {
163-
return BUILD_WRAPPER("org/kiwix/libzim/Entry", THIS->getEntryByTitle(TO_C(index)));
152+
return BUILD_WRAPPER(THIS->getEntryByTitle(TO_C(index)));
164153
}
165154

166155
METHOD(jboolean, hasEntryByTitle, jstring title) {
167156
return TO_JNI(THIS->hasEntryByPath(TO_C(title)));
168157
}
169158

170159
METHOD(jobject, getEntryByClusterOrder, jint index) {
171-
return BUILD_WRAPPER("org/kiwix/libzim/Entry", THIS->getEntryByClusterOrder(TO_C(index)));
160+
return BUILD_WRAPPER(THIS->getEntryByClusterOrder(TO_C(index)));
172161
}
173162

174163
METHOD0(jobject, getMainEntry) {
175-
return BUILD_WRAPPER("org/kiwix/libzim/Entry", THIS->getMainEntry());
164+
return BUILD_WRAPPER(THIS->getMainEntry());
176165
}
177166

178167
GETTER(jboolean, hasMainEntry)
179168

180169
METHOD0(jobject, getRandomEntry) {
181-
return BUILD_WRAPPER("org/kiwix/libzim/Entry", THIS->getRandomEntry());
170+
return BUILD_WRAPPER(THIS->getRandomEntry());
182171
}
183172

184173
GETTER(jboolean, hasFulltextIndex)
@@ -192,62 +181,57 @@ GETTER(jboolean, hasNewNamespaceScheme)
192181
#define ITER_BY_PATH 0
193182
#define ITER_BY_TITLE 1
194183
#define ITER_EFFICIENT 2
184+
185+
186+
// No use of the macro BUILD_WRAPPER as `EntryIterator` takes a integer(I) to
187+
// track the order. So the signature of the constructor is not the same.
188+
// The same way, as we are building a iterator, it has two nativeHandle (for begin and end).
195189
METHOD0(jobject, iterByPath) {
196190
auto range = THIS->iterByPath();
197-
jclass objClass = env->FindClass("org/kiwix/libzim/EntryIterator");
198-
jmethodID initMethod = env->GetMethodID(objClass, "<init>", "(I)V");
199-
jobject obj = env->NewObject(objClass, initMethod, ITER_BY_PATH);
200-
SET_HANDLE(zim::Archive::iterator<zim::EntryOrder::pathOrder>, obj, range.begin());
191+
auto beginIter = NEW_RESOURCE(range.begin());
192+
auto endIter = NEW_RESOURCE(range.end());
201193

202-
auto end_ptr = std::make_shared<zim::Archive::iterator<zim::EntryOrder::pathOrder>>(range.end());
203-
setPtr(env, obj, std::move(end_ptr), "nativeHandleEnd");
204-
return obj;
194+
jclass wrapperClass = env->FindClass("org/kiwix/libzim/EntryIterator");
195+
jmethodID initMethod = env->GetMethodID(wrapperClass, "<init>", "(org/kiwix/Wrapper/Resource;org/kiwix/Wrapper/Resource)V");
196+
return env->NewObject(wrapperClass, initMethod, beginIter, endIter);
205197
}
206198

207199
METHOD0(jobject, iterByTitle) {
208200
auto range = THIS->iterByTitle();
209-
jclass objClass = env->FindClass("org/kiwix/libzim/EntryIterator");
210-
jmethodID initMethod = env->GetMethodID(objClass, "<init>", "(I)V");
211-
jobject obj = env->NewObject(objClass, initMethod, ITER_BY_TITLE);
212-
SET_HANDLE(zim::Archive::iterator<zim::EntryOrder::titleOrder>, obj, range.begin());
201+
auto beginIter = NEW_RESOURCE(range.begin());
202+
auto endIter = NEW_RESOURCE(range.end());
213203

214-
auto end_ptr = std::make_shared<zim::Archive::iterator<zim::EntryOrder::titleOrder>>(range.end());
215-
setPtr(env, obj, std::move(end_ptr), "nativeHandleEnd");
216-
return obj;
204+
jclass wrapperClass = env->FindClass("org/kiwix/libzim/EntryIterator");
205+
jmethodID initMethod = env->GetMethodID(wrapperClass, "<init>", "(org/kiwix/Wrapper/Resource;org/kiwix/Wrapper/Resource)V");
206+
return env->NewObject(wrapperClass, initMethod, beginIter, endIter);
217207
}
218208

219209
METHOD0(jobject, iterEfficient) {
220210
auto range = THIS->iterEfficient();
221-
jclass objClass = env->FindClass("org/kiwix/libzim/EntryIterator");
222-
jmethodID initMethod = env->GetMethodID(objClass, "<init>", "(I)V");
223-
jobject obj = env->NewObject(objClass, initMethod, ITER_EFFICIENT);
224-
SET_HANDLE(zim::Archive::iterator<zim::EntryOrder::efficientOrder>, obj, range.begin());
211+
auto beginIter = NEW_RESOURCE(range.begin());
212+
auto endIter = NEW_RESOURCE(range.end());
225213

226-
auto end_ptr = std::make_shared<zim::Archive::iterator<zim::EntryOrder::efficientOrder>>(range.end());
227-
setPtr(env, obj, std::move(end_ptr), "nativeHandleEnd");
228-
return obj;
214+
jclass wrapperClass = env->FindClass("org/kiwix/libzim/EntryIterator");
215+
jmethodID initMethod = env->GetMethodID(wrapperClass, "<init>", "(org/kiwix/Wrapper/Resource;org/kiwix/Wrapper/Resource)V");
216+
return env->NewObject(wrapperClass, initMethod, beginIter, endIter);
229217
}
230218

231219
METHOD(jobject, findByPath, jstring path) {
232220
auto range = THIS->findByPath(TO_C(path));
233-
jclass objClass = env->FindClass("org/kiwix/libzim/EntryIterator");
234-
jmethodID initMethod = env->GetMethodID(objClass, "<init>", "(I)V");
235-
jobject obj = env->NewObject(objClass, initMethod, ITER_BY_PATH);
236-
SET_HANDLE(zim::Archive::iterator<zim::EntryOrder::pathOrder>, obj, range.begin());
221+
auto beginIter = NEW_RESOURCE(range.begin());
222+
auto endIter = NEW_RESOURCE(range.end());
237223

238-
auto end_ptr = std::make_shared<zim::Archive::iterator<zim::EntryOrder::pathOrder>>(range.end());
239-
setPtr(env, obj, std::move(end_ptr), "nativeHandleEnd");
240-
return obj;
224+
jclass wrapperClass = env->FindClass("org/kiwix/libzim/EntryIterator");
225+
jmethodID initMethod = env->GetMethodID(wrapperClass, "<init>", "(org/kiwix/Wrapper/Resource;org/kiwix/Wrapper/Resource)V");
226+
return env->NewObject(wrapperClass, initMethod, beginIter, endIter);
241227
}
242228

243229
METHOD(jobject, findByTitle, jstring title) {
244230
auto range = THIS->findByTitle(TO_C(title));
245-
jclass objClass = env->FindClass("org/kiwix/libzim/EntryIterator");
246-
jmethodID initMethod = env->GetMethodID(objClass, "<init>", "(I)V");
247-
jobject obj = env->NewObject(objClass, initMethod, ITER_BY_TITLE);
248-
SET_HANDLE(zim::Archive::iterator<zim::EntryOrder::titleOrder>, obj, range.begin());
249-
250-
auto end_ptr = std::make_shared<zim::Archive::iterator<zim::EntryOrder::titleOrder>>(range.end());
251-
setPtr(env, obj, std::move(end_ptr), "nativeHandleEnd");
252-
return obj;
231+
auto beginIter = NEW_RESOURCE(range.begin());
232+
auto endIter = NEW_RESOURCE(range.end());
233+
234+
jclass wrapperClass = env->FindClass("org/kiwix/libzim/EntryIterator");
235+
jmethodID initMethod = env->GetMethodID(wrapperClass, "<init>", "(org/kiwix/Wrapper/Resource;org/kiwix/Wrapper/Resource)V");
236+
return env->NewObject(wrapperClass, initMethod, beginIter, endIter);
253237
}

0 commit comments

Comments
 (0)