You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2013/10/09 15:47:20 UTC
svn commit: r1530607 - in /subversion/trunk/subversion/bindings/javahl:
native/ tests/org/apache/subversion/javahl/
Author: brane
Date: Wed Oct 9 13:47:20 2013
New Revision: 1530607
URL: http://svn.apache.org/r1530607
Log:
Fix a construction ordering bug in JavaHL that could lead to a crash during
finalization of RemoteSession objects.
[in subversion/bindings/javahl/native]
* RemoteSessionContext.h (RemoteSessionContext::RemoteSessionContext):
Remove the contextHolder and jprogress parameters.
(RemoteSessionContext::activate): New function.
* RemoteSessionContext.cpp (RemoteSessionContext::RemoteSessionContext):
Do not attach to the Java object and do not set the progress callback.
(RemoteSessionContext::activate): Instaed, attach and set callback here.
* OperationContext.cpp (OperationContext::progress): Check if the Java context
was set, and revert to no-op if it wasn't.
* RemoteSession.h (RemoteSession::RemoteSession): Changed signature:
the constuctor does not create its Java counterpart, nor set up
the progress callback in same.
* RemoteSession.cpp (RemoteSession::open): Create the Java object and attach
the session context after the native object was successfully created.
(RemoteSession::RemoteSession): Do not create the Java object in the
constructor.
* org_apache_subversion_javahl_remote_RemoteFactory.cpp
(Java_org_apache_subversion_javahl_remote_RemoteFactory_open):
Call JNIEntryStatic instead of faking it through JNIEntry.
[in subversion/bindings/javahl/tests/org/apache/subversion/javahl]
* SVNRemoteTests.java (SVNRemoteTests.testSessionGC):
New test case, checks the sequence of events that led to a crach.
Modified:
subversion/trunk/subversion/bindings/javahl/native/OperationContext.cpp
subversion/trunk/subversion/bindings/javahl/native/RemoteSession.cpp
subversion/trunk/subversion/bindings/javahl/native/RemoteSession.h
subversion/trunk/subversion/bindings/javahl/native/RemoteSessionContext.cpp
subversion/trunk/subversion/bindings/javahl/native/RemoteSessionContext.h
subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_remote_RemoteFactory.cpp
subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/SVNRemoteTests.java
Modified: subversion/trunk/subversion/bindings/javahl/native/OperationContext.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/OperationContext.cpp?rev=1530607&r1=1530606&r2=1530607&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/OperationContext.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/OperationContext.cpp Wed Oct 9 13:47:20 2013
@@ -328,6 +328,9 @@ OperationContext::progress(apr_off_t pro
apr_pool_t *pool)
{
jobject jctx = (jobject) baton;
+ if (!jctx)
+ return;
+
JNIEnv *env = JNIUtil::getEnv();
// Create a local frame for our references
Modified: subversion/trunk/subversion/bindings/javahl/native/RemoteSession.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/RemoteSession.cpp?rev=1530607&r1=1530606&r2=1530607&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/RemoteSession.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/RemoteSession.cpp Wed Oct 9 13:47:20 2013
@@ -137,17 +137,51 @@ RemoteSession::open(jint jretryAttempts,
initialized = true;
}
- jobject jthis_out = NULL;
RemoteSession* session = new RemoteSession(
- &jthis_out, jretryAttempts, url, uuid,
- configDirectory,
- usernameStr, passwordStr, prompter, jprogress);
+ jretryAttempts, url, uuid, configDirectory,
+ usernameStr, passwordStr, prompter);
if (JNIUtil::isJavaExceptionThrown() || !session)
{
delete session;
- jthis_out = NULL;
+ return NULL;
}
- return jthis_out;
+
+ // Create java session object
+ JNIEnv *env = JNIUtil::getEnv();
+
+ jclass clazz = env->FindClass(JAVA_CLASS_REMOTE_SESSION);
+ if (JNIUtil::isJavaExceptionThrown())
+ {
+ delete session;
+ return NULL;
+ }
+
+ static jmethodID ctor = 0;
+ if (ctor == 0)
+ {
+ ctor = env->GetMethodID(clazz, "<init>", "(J)V");
+ if (JNIUtil::isJavaExceptionThrown())
+ {
+ delete session;
+ return NULL;
+ }
+ }
+
+ jobject jremoteSession = env->NewObject(clazz, ctor, session->getCppAddr());
+ if (JNIUtil::isJavaExceptionThrown())
+ {
+ delete session;
+ return NULL;
+ }
+
+ session->m_context->activate(jremoteSession, jprogress);
+ if (JNIUtil::isJavaExceptionThrown())
+ {
+ delete session;
+ jremoteSession = NULL;
+ }
+
+ return jremoteSession;
}
@@ -163,37 +197,15 @@ namespace{
typedef std::pair<attempt_set::iterator, bool> attempt_insert;
} // anonymous namespace
-RemoteSession::RemoteSession(jobject* jthis_out, int retryAttempts,
+RemoteSession::RemoteSession(int retryAttempts,
const char* url, const char* uuid,
const char* configDirectory,
const char* username, const char* password,
- Prompter*& prompter, jobject jprogress)
+ Prompter*& prompter)
: m_session(NULL), m_context(NULL)
{
- // Create java session object
- JNIEnv *env = JNIUtil::getEnv();
-
- jclass clazz = env->FindClass(JAVA_CLASS_REMOTE_SESSION);
- if (JNIUtil::isJavaExceptionThrown())
- return;
-
- static jmethodID ctor = 0;
- if (ctor == 0)
- {
- ctor = env->GetMethodID(clazz, "<init>", "(J)V");
- if (JNIUtil::isJavaExceptionThrown())
- return;
- }
-
- jlong cppAddr = this->getCppAddr();
-
- jobject jremoteSession = env->NewObject(clazz, ctor, cppAddr);
- if (JNIUtil::isJavaExceptionThrown())
- return;
-
m_context = new RemoteSessionContext(
- jremoteSession, pool, configDirectory,
- username, password, prompter, jprogress);
+ pool, configDirectory, username, password, prompter);
if (JNIUtil::isJavaExceptionThrown())
return;
@@ -232,6 +244,8 @@ RemoteSession::RemoteSession(jobject* jt
if (cycle_detected)
{
+ JNIEnv *env = JNIUtil::getEnv();
+
jstring exmsg = JNIUtil::makeJString(
apr_psprintf(pool.getPool(),
_("Redirect cycle detected for URL '%s'"),
@@ -257,6 +271,8 @@ RemoteSession::RemoteSession(jobject* jt
if (corrected_url)
{
+ JNIEnv *env = JNIUtil::getEnv();
+
jstring exmsg = JNIUtil::makeJString(_("Too many redirects"));
if (JNIUtil::isJavaExceptionThrown())
return;
@@ -282,8 +298,6 @@ RemoteSession::RemoteSession(jobject* jt
env->Throw(static_cast<jthrowable>(ex));
return;
}
-
- *jthis_out = jremoteSession;
}
RemoteSession::~RemoteSession()
Modified: subversion/trunk/subversion/bindings/javahl/native/RemoteSession.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/RemoteSession.h?rev=1530607&r1=1530606&r2=1530607&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/RemoteSession.h (original)
+++ subversion/trunk/subversion/bindings/javahl/native/RemoteSession.h Wed Oct 9 13:47:20 2013
@@ -113,11 +113,11 @@ class RemoteSession : public SVNBase
private:
friend class CommitEditor;
- RemoteSession(jobject*, int retryAttempts,
+ RemoteSession(int retryAttempts,
const char* url, const char* uuid,
const char* configDirectory,
const char* username, const char* password,
- Prompter*& prompter, jobject jprogress);
+ Prompter*& prompter);
svn_ra_session_t* m_session;
RemoteSessionContext* m_context;
Modified: subversion/trunk/subversion/bindings/javahl/native/RemoteSessionContext.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/RemoteSessionContext.cpp?rev=1530607&r1=1530606&r2=1530607&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/RemoteSessionContext.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/RemoteSessionContext.cpp Wed Oct 9 13:47:20 2013
@@ -29,13 +29,11 @@
#include "JNIUtil.h"
#include "Prompter.h"
-#define STRING_RETURN_SIGNATURE "()Ljava/lang/String;"
RemoteSessionContext::RemoteSessionContext(
- jobject contextHolder, SVN::Pool &pool,
- const char* configDirectory,
- const char* usernameStr, const char* passwordStr,
- Prompter* prompter, jobject jprogress)
+ SVN::Pool &pool, const char* configDirectory,
+ const char* usernameStr, const char* passwordStr,
+ Prompter* prompter)
: OperationContext(pool), m_raCallbacks(NULL)
{
setConfigDirectory(configDirectory);
@@ -48,32 +46,6 @@ RemoteSessionContext::RemoteSessionConte
setPrompt(prompter);
/*
- * Attach session context java object
- */
- static jfieldID ctxFieldID = 0;
- attachJavaObject(contextHolder,
- "L"JAVA_PACKAGE"/remote/RemoteSession$RemoteSessionContext;",
- "sessionContext", &ctxFieldID);
-
- /*
- * Set the progress callback
- */
- JNIEnv *env = JNIUtil::getEnv();
-
- jclass clazz = env->GetObjectClass(m_jctx);
- if (JNIUtil::isJavaExceptionThrown())
- return;
-
- jmethodID mid = env->GetMethodID(
- clazz, "setProgressCallback",
- "(L"JAVA_PACKAGE"/callback/ProgressCallback;)V");
- if (JNIUtil::isJavaExceptionThrown() || mid == 0)
- return;
-
- env->CallVoidMethod(m_jctx, mid, jprogress);
- env->DeleteLocalRef(jprogress);
-
- /*
* Setup callbacks
*/
SVN_JNI_ERR(svn_ra_create_callbacks(&m_raCallbacks, m_pool->getPool()), );
@@ -81,7 +53,7 @@ RemoteSessionContext::RemoteSessionConte
m_raCallbacks->auth_baton = getAuthBaton(pool);
m_raCallbacks->cancel_func = checkCancel;
m_raCallbacks->get_client_string = clientName;
- m_raCallbacks->progress_baton = m_jctx;
+ m_raCallbacks->progress_baton = NULL;
m_raCallbacks->progress_func = progress;
/*
@@ -102,6 +74,36 @@ RemoteSessionContext::~RemoteSessionCont
{
}
+void RemoteSessionContext::activate(jobject jremoteSession, jobject jprogress)
+{
+ /*
+ * Attach session context java object
+ */
+ static jfieldID ctxFieldID = 0;
+ attachJavaObject(jremoteSession,
+ "L"JAVA_PACKAGE"/remote/RemoteSession$RemoteSessionContext;",
+ "sessionContext", &ctxFieldID);
+
+ /*
+ * Set the progress callback
+ */
+ JNIEnv *env = JNIUtil::getEnv();
+
+ jclass clazz = env->GetObjectClass(m_jctx);
+ if (JNIUtil::isJavaExceptionThrown())
+ return;
+
+ jmethodID mid = env->GetMethodID(
+ clazz, "setProgressCallback",
+ "(L"JAVA_PACKAGE"/callback/ProgressCallback;)V");
+ if (JNIUtil::isJavaExceptionThrown() || mid == 0)
+ return;
+
+ env->CallVoidMethod(m_jctx, mid, jprogress);
+ env->DeleteLocalRef(jprogress);
+ m_raCallbacks->progress_baton = m_jctx;
+}
+
void *
RemoteSessionContext::getCallbackBaton()
{
Modified: subversion/trunk/subversion/bindings/javahl/native/RemoteSessionContext.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/RemoteSessionContext.h?rev=1530607&r1=1530606&r2=1530607&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/RemoteSessionContext.h (original)
+++ subversion/trunk/subversion/bindings/javahl/native/RemoteSessionContext.h Wed Oct 9 13:47:20 2013
@@ -34,11 +34,12 @@
class RemoteSessionContext : public OperationContext
{
public:
- RemoteSessionContext(jobject contextHolder, SVN::Pool &pool,
+ RemoteSessionContext(SVN::Pool &pool,
const char* jconfigDirectory,
const char* jusername, const char* jpassword,
- Prompter* prompter, jobject jprogress);
+ Prompter* prompter);
virtual ~RemoteSessionContext();
+ void activate(jobject jremoteSession, jobject jprogress);
void * getCallbackBaton();
svn_ra_callbacks2_t* getCallbacks();
Modified: subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_remote_RemoteFactory.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_remote_RemoteFactory.cpp?rev=1530607&r1=1530606&r2=1530607&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_remote_RemoteFactory.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_remote_RemoteFactory.cpp Wed Oct 9 13:47:20 2013
@@ -36,15 +36,14 @@
JNIEXPORT jobject JNICALL
Java_org_apache_subversion_javahl_remote_RemoteFactory_open(
- JNIEnv *env, jclass jclass, jint jretryAttempts,
+ JNIEnv *env, jclass jclazz, jint jretryAttempts,
jstring jurl, jstring juuid,
jstring jconfigDirectory,
jstring jusername, jstring jpassword,
jobject jprompter, jobject jprogress)
{
//JNI macros need jthis but this is a static call
- jobject jthis = NULL;
- JNIEntry(Remotefactory, open);
+ JNIEntryStatic(RemoteFactory, open);
/*
* Create RemoteSession C++ object and return its java wrapper to the caller
Modified: subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/SVNRemoteTests.java
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/SVNRemoteTests.java?rev=1530607&r1=1530606&r2=1530607&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/SVNRemoteTests.java (original)
+++ subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/SVNRemoteTests.java Wed Oct 9 13:47:20 2013
@@ -128,6 +128,43 @@ public class SVNRemoteTests extends SVNT
session.dispose();
}
+ public void testSessionGC() throws Exception
+ {
+ int svnErrorCode = 0;
+ try {
+ try {
+ String prefix = getTestRepoUrl().substring(
+ 0, 1 + getTestRepoUrl().lastIndexOf("/"));
+ new RemoteFactory(
+ super.conf.getAbsolutePath(),
+ USERNAME, PASSWORD,
+ new DefaultPromptUserPassword(), null)
+ .openRemoteSession(prefix + "repositorydoesnotexisthere");
+ }
+ finally
+ {
+ for(int i = 0; i < 100; i++)
+ {
+ Runtime.getRuntime().gc(); // GC should run finalize
+
+ // Do something
+ byte[] memEater = new byte[1024 * 1024];
+ Arrays.fill(memEater, (byte) i);
+
+ // Do some more javahl activity (this url is OK)
+ final ISVNRemote session = getSession();
+ session.getLatestRevision();
+ session.dispose();
+ }
+ }
+ }
+ catch (ClientException ex)
+ {
+ svnErrorCode = ex.getAllMessages().get(0).getCode();
+ }
+ assertEquals(180001, svnErrorCode);
+ }
+
public void testDatedRev() throws Exception
{
ISVNRemote session = getSession();