You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2011/08/05 10:22:08 UTC

svn commit: r1154119 - in /subversion/trunk/subversion/bindings/javahl/native: ClientContext.cpp ClientContext.h JNIUtil.cpp JNIUtil.h Pool.cpp Pool.h SVNBase.cpp SVNBase.h SVNClient.cpp

Author: rhuijben
Date: Fri Aug  5 08:22:07 2011
New Revision: 1154119

URL: http://svn.apache.org/viewvc?rev=1154119&view=rev
Log:
In JavaHL: Create a per SVNClient pool to hold things like the client and
working copy context. Do this to allow releasing their memory with the
SVNClient.

Also remove the global pool mutex as creating an apr subpool is thread safe
when the pool allocator is thread safe (default setting). The request pool
handling which was guarded by the same critical section is per thread,
so this doesn't need guarding.

* subversion/bindings/javahl/native/ClientContext.cpp
  (ClientContext::ClientContext): Use passed pool instead of global pool.
    Remove mutex handling.

* subversion/bindings/javahl/native/ClientContext.h
  (ClientContext::ClientContext): Add argument.

* subversion/bindings/javahl/native/JNIUtil.cpp
  (JNIUtil::g_globalPoolMutext): Remove.
  (JNIUtil::JNIGlobalInit): Stop initializing pool mutex.
  (JNIUtil::getGlobalPoolMutex): Remove.

* subversion/bindings/javahl/native/JNIUtil.h
  (JNIUtil::g_globalPoolMutext,
   JNIUtil::getGlobalPoolMutex): Remove.

* subversion/bindings/javahl/native/Pool.cpp
  (SVN::Pool::Pool): Add variants to create subpools.
  (SVN::Pool::~Pool):
  (SVN::Pool::getPool): New function.

* subversion/bindings/javahl/native/Pool.h
  (SVN::Pool::Pool): Add variants.
  (SVN::Pool::getPool): Add function.
  (SVN::Pool): Update comments. Add boolean.

* subversion/bindings/javahl/native/SVNBase.cpp
  (SVNBase::SVNBase): Initialize subpool as child of the global pool.

* subversion/bindings/javahl/native/SVNBase.h
  (SVNBase): Add subpool.

* subversion/bindings/javahl/native/SVNClient.cpp
  (SVNClient::SVNClient): Pass pool to client.

Modified:
    subversion/trunk/subversion/bindings/javahl/native/ClientContext.cpp
    subversion/trunk/subversion/bindings/javahl/native/ClientContext.h
    subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
    subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h
    subversion/trunk/subversion/bindings/javahl/native/Pool.cpp
    subversion/trunk/subversion/bindings/javahl/native/Pool.h
    subversion/trunk/subversion/bindings/javahl/native/SVNBase.cpp
    subversion/trunk/subversion/bindings/javahl/native/SVNBase.h
    subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp

Modified: subversion/trunk/subversion/bindings/javahl/native/ClientContext.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/ClientContext.cpp?rev=1154119&r1=1154118&r2=1154119&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/ClientContext.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/ClientContext.cpp Fri Aug  5 08:22:07 2011
@@ -38,12 +38,11 @@
 #include "CommitMessage.h"
 
 
-ClientContext::ClientContext(jobject jsvnclient)
+ClientContext::ClientContext(jobject jsvnclient, SVN::Pool &pool)
     : m_prompter(NULL),
       m_cancelOperation(false)
 {
     JNIEnv *env = JNIUtil::getEnv();
-    JNICriticalSection criticalSection(*JNIUtil::getGlobalPoolMutex());
 
     /* Grab a global reference to the Java object embedded in the parent Java
        object. */
@@ -72,8 +71,7 @@ ClientContext::ClientContext(jobject jsv
 
     env->DeleteLocalRef(jctx);
 
-    /* Create a long-lived client context object in the global pool. */
-    SVN_JNI_ERR(svn_client_create_context(&persistentCtx, JNIUtil::getPool()),
+    SVN_JNI_ERR(svn_client_create_context(&persistentCtx, pool.getPool()),
                 );
 
     /* None of the following members change during the lifetime of

Modified: subversion/trunk/subversion/bindings/javahl/native/ClientContext.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/ClientContext.h?rev=1154119&r1=1154118&r2=1154119&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/ClientContext.h (original)
+++ subversion/trunk/subversion/bindings/javahl/native/ClientContext.h Fri Aug  5 08:22:07 2011
@@ -70,7 +70,7 @@ class ClientContext
                                                  apr_pool_t *pool);
 
  public:
-  ClientContext(jobject jsvnclient);
+  ClientContext(jobject jsvnclient, SVN::Pool &pool);
   ~ClientContext();
 
   static svn_error_t *checkCancel(void *cancelBaton);

Modified: subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp?rev=1154119&r1=1154118&r2=1154119&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp Fri Aug  5 08:22:07 2011
@@ -64,7 +64,6 @@ apr_pool_t *JNIUtil::g_pool = NULL;
 std::list<SVNBase*> JNIUtil::g_finalizedObjects;
 JNIMutex *JNIUtil::g_finalizedObjectsMutex = NULL;
 JNIMutex *JNIUtil::g_logMutex = NULL;
-JNIMutex *JNIUtil::g_globalPoolMutext = NULL;
 bool JNIUtil::g_initException;
 bool JNIUtil::g_inInit;
 JNIEnv *JNIUtil::g_initEnv;
@@ -250,10 +249,6 @@ bool JNIUtil::JNIGlobalInit(JNIEnv *env)
   if (isExceptionThrown())
     return false;
 
-  g_globalPoolMutext = new JNIMutex(g_pool);
-  if (isExceptionThrown())
-    return false;
-
   // initialized the thread local storage
   if (!JNIThreadData::initThreadData())
     return false;
@@ -276,15 +271,6 @@ apr_pool_t *JNIUtil::getPool()
   return g_pool;
 }
 
-/**
- * Return the mutex securing the global pool.
- * @return the mutex for the global pool
- */
-JNIMutex *JNIUtil::getGlobalPoolMutex()
-{
-  return g_globalPoolMutext;
-}
-
 void JNIUtil::raiseThrowable(const char *name, const char *message)
 {
   if (getLogLevel() >= errorLog)

Modified: subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h?rev=1154119&r1=1154118&r2=1154119&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h (original)
+++ subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h Fri Aug  5 08:22:07 2011
@@ -135,7 +135,6 @@ class JNIUtil
   static apr_pool_t *getPool();
   static bool JNIGlobalInit(JNIEnv *env);
   static bool JNIInit(JNIEnv *env);
-  static JNIMutex *getGlobalPoolMutex();
   enum { formatBufferSize = 2048 };
   enum { noLog, errorLog, exceptionLog, entryLog } LogLevel;
 
@@ -202,11 +201,6 @@ class JNIUtil
    * The stream to write log messages to.
    */
   static std::ofstream g_logStream;
-
-  /**
-   * Flag to secure our global pool.
-   */
-  static JNIMutex *g_globalPoolMutext;
 };
 
 /**

Modified: subversion/trunk/subversion/bindings/javahl/native/Pool.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/Pool.cpp?rev=1154119&r1=1154118&r2=1154119&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/Pool.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/Pool.cpp Fri Aug  5 08:22:07 2011
@@ -36,9 +36,21 @@
  */
 SVN::Pool::Pool()
 {
-  JNICriticalSection criticalSection(*JNIUtil::getGlobalPoolMutex());
   m_pool = svn_pool_create(JNIUtil::getPool());
   JNIUtil::setRequestPool(this);
+  m_request_pool = true;
+}
+
+SVN::Pool::Pool(const Pool &parent_pool)
+{
+  m_pool = svn_pool_create(parent_pool.m_pool);
+  m_request_pool = false;
+}
+
+SVN::Pool::Pool(apr_pool_t *parent_pool)
+{
+  m_pool = svn_pool_create(parent_pool);
+  m_request_pool = false;
 }
 
 /**
@@ -47,8 +59,17 @@ SVN::Pool::Pool()
  */
 SVN::Pool::~Pool()
 {
-  JNICriticalSection criticalSection(*JNIUtil::getGlobalPoolMutex());
-  JNIUtil::setRequestPool(NULL);
+  if (m_request_pool)
+    JNIUtil::setRequestPool(NULL);
+
   if (m_pool)
-    svn_pool_destroy(m_pool);
+    {
+      svn_pool_destroy(m_pool);
+      m_pool = NULL;
+    }
+}
+
+apr_pool_t* SVN::Pool::getPool()
+{
+  return m_pool;
 }

Modified: subversion/trunk/subversion/bindings/javahl/native/Pool.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/Pool.h?rev=1154119&r1=1154118&r2=1154119&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/Pool.h (original)
+++ subversion/trunk/subversion/bindings/javahl/native/Pool.h Fri Aug  5 08:22:07 2011
@@ -39,10 +39,15 @@ namespace SVN {
   {
   public:
     Pool();
+    Pool(const Pool &parent_pool);
+    Pool(apr_pool_t *parent_pool);
     ~Pool();
     apr_pool_t *pool() const;
     void clear() const;
 
+  public:
+    apr_pool_t* getPool();
+
   private:
     /**
      * The apr pool request pool.
@@ -50,14 +55,20 @@ namespace SVN {
     apr_pool_t *m_pool;
 
     /**
-     * We declare the copy constructor and assignment operator private
-     * here, so that the compiler won't inadvertently use them for us.
-     * The default copy constructor just copies all the data members,
-     * which would create two pointers to the same pool, one of which
-     * would get destroyed while the other thought it was still
-     * valid...and BOOM!  Hence the private declaration.
+     * Boolean indicating whether this pool is a request pool vs just a normal
+     * subpool.
+     */
+    bool m_request_pool;
+
+    /**
+     * We declare the assignment operator private here, so that the compiler
+     * won't inadvertently use them for us.
+     * The default code just copies all the data members, which would create
+     * two pointers to the same pool, one of which would get destroyed while
+     * the other thought it was still valid...and BOOM!
+     *
+     * Hence the private declaration.
      */
-    Pool(Pool &that);
     Pool &operator=(Pool &that);
   };
 

Modified: subversion/trunk/subversion/bindings/javahl/native/SVNBase.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/SVNBase.cpp?rev=1154119&r1=1154118&r2=1154119&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/SVNBase.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/SVNBase.cpp Fri Aug  5 08:22:07 2011
@@ -28,6 +28,7 @@
 #include "JNIUtil.h"
 
 SVNBase::SVNBase()
+    : pool(JNIUtil::getPool())
 {
   jthis = NULL;
 }

Modified: subversion/trunk/subversion/bindings/javahl/native/SVNBase.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/SVNBase.h?rev=1154119&r1=1154118&r2=1154119&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/SVNBase.h (original)
+++ subversion/trunk/subversion/bindings/javahl/native/SVNBase.h Fri Aug  5 08:22:07 2011
@@ -28,6 +28,7 @@
 #define SVNBASE_H
 
 #include <jni.h>
+#include "Pool.h"
 
 class SVNBase
 {
@@ -96,6 +97,9 @@ class SVNBase
    */
   static void findCppAddrFieldID(jfieldID *fid, const char *className,
                                  JNIEnv *env);
+
+protected:
+    SVN::Pool pool;
 };
 
 #endif // SVNBASE_H

Modified: subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp?rev=1154119&r1=1154118&r2=1154119&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp Fri Aug  5 08:22:07 2011
@@ -70,7 +70,7 @@
 
 
 SVNClient::SVNClient(jobject jthis_in)
-    : context(jthis_in)
+    : context(jthis_in, pool)
 {
 }