Greetings,
Currently SVNBase class uses a member variable jthis to hold a copy of
jobject
reference. This variable lives as long as SVNClient object lives, however,
the
reference itself is only valid during a single JNI call. To address this
mismatch
the attached patch passes the jobject reference as a parameter instead.
This
eliminates the risk of the jobject reference being misused outside the
scope
where it is valid. This mismatch becomes more evident on JavaHL
RA editor API is implemented where objects like directories live across
method
calls.
[[[
JavaHL: Explicitly pass jobject jthis when processing dispose() call rather
than stashing a reference in the SVNBase class where it can be misused
later
[ in subversion/bindings/javahl/native ]
* SVNBase.cpp,
SVNBase.h
(dispose, jthis): Accept jobject jthis as explicit parameter to dispose()
and
delete the member variable jthis
* SVNClient.cpp,
SVNClient.h,
SVNRepos.cpp,
SVNRepos.h
(dispose): Accept object jthis as explicit parameter and pass it to
SVNBase::dispose
* org_apache_subversion_javahl_SVNClient.cpp,
org_apache_subversion_javahl_SVNRepos.cpp
(Java_org_apache_subversion_javahl_SVNClient_dispose,
Java_org_apache_subversion_javahl_SVNRepos_dispose):
Pass object jthis as explicit parameter and pass it to the C++ wrapper
class
]]]
Thank you,
Vladimir
Index: subversion/bindings/javahl/native/SVNRepos.cpp
===================================================================
--- subversion/bindings/javahl/native/SVNRepos.cpp (revision 1328758)
+++ subversion/bindings/javahl/native/SVNRepos.cpp (working copy)
@@ -54,10 +54,10 @@ SVNRepos *SVNRepos::getCppObject(jobject jthis)
return (cppAddr == 0 ? NULL : reinterpret_cast<SVNRepos *>(cppAddr));
}
-void SVNRepos::dispose()
+void SVNRepos::dispose(jobject jthis)
{
static jfieldID fid = 0;
- SVNBase::dispose(&fid, JAVA_PACKAGE"/SVNRepos");
+ SVNBase::dispose(jthis, &fid, JAVA_PACKAGE"/SVNRepos");
}
void SVNRepos::cancelOperation()
Index: subversion/bindings/javahl/native/SVNClient.h
===================================================================
--- subversion/bindings/javahl/native/SVNClient.h (revision 1328758)
+++ subversion/bindings/javahl/native/SVNClient.h (working copy)
@@ -196,7 +196,7 @@ class SVNClient :public SVNBase
ClientContext &getClientContext();
const char *getLastPath();
- void dispose();
+ void dispose(jobject jthis);
static SVNClient *getCppObject(jobject jthis);
SVNClient(jobject jthis_in);
virtual ~SVNClient();
Index:
subversion/bindings/javahl/native/org_apache_subversion_javahl_SVNRepos.cpp
===================================================================
--- subversion/bindings/javahl/native/org_apache_subversion_javahl_SVNRepos.cpp
(revision 1328758)
+++ subversion/bindings/javahl/native/org_apache_subversion_javahl_SVNRepos.cpp
(working copy)
@@ -60,7 +60,7 @@ Java_org_apache_subversion_javahl_SVNRepos_dispose
JNIUtil::throwError(_("bad C++ this"));
return;
}
- cl->dispose();
+ cl->dispose(jthis);
}
JNIEXPORT void JNICALL
Index: subversion/bindings/javahl/native/SVNBase.cpp
===================================================================
--- subversion/bindings/javahl/native/SVNBase.cpp (revision 1328758)
+++ subversion/bindings/javahl/native/SVNBase.cpp (working copy)
@@ -30,7 +30,6 @@
SVNBase::SVNBase()
: pool(JNIUtil::getPool())
{
- jthis = NULL;
}
SVNBase::~SVNBase()
@@ -58,17 +57,6 @@ jlong SVNBase::findCppAddrForJObject(jobject jthis
if (JNIUtil::isJavaExceptionThrown())
return 0;
- if (cppAddr)
- {
- /* jthis is not guaranteed to be the same between JNI invocations, so
- we do a little dance here and store the updated version in our
- object for this invocation.
-
- findCppAddrForJObject() is, by necessity, called before any other
- methods on the C++ object, so by doing this we can guarantee a
- valid jthis pointer for subsequent uses. */
- (reinterpret_cast<SVNBase *> (cppAddr))->jthis = jthis;
- }
return cppAddr;
}
}
@@ -82,17 +70,15 @@ void SVNBase::finalize()
JNIUtil::enqueueForDeletion(this);
}
-void SVNBase::dispose(jfieldID *fid, const char *className)
+void SVNBase::dispose(jobject jthis, jfieldID *fid, const char *className)
{
- jobject my_jthis = this->jthis;
-
delete this;
JNIEnv *env = JNIUtil::getEnv();
SVNBase::findCppAddrFieldID(fid, className, env);
if (*fid == 0)
return;
- env->SetLongField(my_jthis, *fid, 0);
+ env->SetLongField(jthis, *fid, 0);
if (JNIUtil::isJavaExceptionThrown())
return;
}
Index:
subversion/bindings/javahl/native/org_apache_subversion_javahl_SVNClient.cpp
===================================================================
---
subversion/bindings/javahl/native/org_apache_subversion_javahl_SVNClient.cpp
(revision 1328758)
+++
subversion/bindings/javahl/native/org_apache_subversion_javahl_SVNClient.cpp
(working copy)
@@ -75,7 +75,7 @@ Java_org_apache_subversion_javahl_SVNClient_dispos
JNIUtil::throwError(_("bad C++ this"));
return;
}
- cl->dispose();
+ cl->dispose(jthis);
}
JNIEXPORT void JNICALL
Index: subversion/bindings/javahl/native/SVNBase.h
===================================================================
--- subversion/bindings/javahl/native/SVNBase.h (revision 1328758)
+++ subversion/bindings/javahl/native/SVNBase.h (working copy)
@@ -49,7 +49,7 @@ class SVNBase
*
* @since 1.4.0
*/
- virtual void dispose() = 0;
+ virtual void dispose(jobject jthis) = 0;
/**
* This method should never be called, as @c dispose() should be
@@ -80,14 +80,8 @@ class SVNBase
*
* @since 1.4.0
*/
- void dispose(jfieldID *fid, const char *className);
+ void dispose(jobject jthis, jfieldID *fid, const char *className);
- /**
- * A pointer to the parent java object. This is not valid across JNI
- * method invocations, and so should be set in each one.
- */
- jobject jthis;
-
private:
/**
* If the value pointed to by @a fid is zero, find the @c jfieldID
Index: subversion/bindings/javahl/native/SVNClient.cpp
===================================================================
--- subversion/bindings/javahl/native/SVNClient.cpp (revision 1328758)
+++ subversion/bindings/javahl/native/SVNClient.cpp (working copy)
@@ -86,10 +86,10 @@ SVNClient *SVNClient::getCppObject(jobject jthis)
return (cppAddr == 0 ? NULL : reinterpret_cast<SVNClient *>(cppAddr));
}
-void SVNClient::dispose()
+void SVNClient::dispose(jobject jthis)
{
static jfieldID fid = 0;
- SVNBase::dispose(&fid, JAVA_PACKAGE"/SVNClient");
+ SVNBase::dispose(jthis, &fid, JAVA_PACKAGE"/SVNClient");
}
jstring SVNClient::getAdminDirectoryName()
Index: subversion/bindings/javahl/native/SVNRepos.h
===================================================================
--- subversion/bindings/javahl/native/SVNRepos.h (revision 1328758)
+++ subversion/bindings/javahl/native/SVNRepos.h (working copy)
@@ -69,7 +69,7 @@ class SVNRepos : public SVNBase
void pack(File &path, ReposNotifyCallback *callback);
SVNRepos();
virtual ~SVNRepos();
- void dispose();
+ void dispose(jobject jthis);
static SVNRepos *getCppObject(jobject jthis);
static svn_error_t *checkCancel(void *cancelBaton);
