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 2015/03/08 02:22:15 UTC

svn commit: r1664939 - /subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp

Author: rhuijben
Date: Sun Mar  8 01:22:15 2015
New Revision: 1664939

URL: http://svn.apache.org/r1664939
Log:
Following up on r1664938, add some debug code to make 'Java error leaks'
visible in maintainer/test mode.

* subversion/bindings/javahl/native/JNIUtil.cpp
  (WrappedException): Track whether the exception is fetched. Use DBG output
    if it isn't.

Modified:
    subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp

Modified: subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp?rev=1664939&r1=1664938&r2=1664939&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp Sun Mar  8 01:22:15 2015
@@ -1079,6 +1079,9 @@ class WrappedException
 {
   JNIEnv *m_env;
   jthrowable m_exception;
+#ifdef SVN_DEBUG
+  bool m_fetched;
+#endif
 public:
   WrappedException(JNIEnv *env)
   {
@@ -1092,6 +1095,8 @@ public:
 
     // As adding a reference in exception state fails
     m_exception = static_cast<jthrowable>(env->NewGlobalRef(exceptionObj));
+
+    m_fetched = false;
   }
 
   static jthrowable get_exception(apr_pool_t *pool)
@@ -1103,6 +1108,9 @@ public:
 
           if (we)
           {
+#ifdef SVN_DEBUG
+              we->m_fetched = TRUE;
+#endif
               // Create reference in local frame, as the pool will be cleared
               return static_cast<jthrowable>(
                             we->m_env->NewLocalRef(we->m_exception));
@@ -1114,6 +1122,10 @@ public:
 private:
   ~WrappedException()
   {
+#ifdef SVN_DEBUG
+      if (!m_fetched)
+          SVN_DBG(("Cleared svn_error_t * before Java exception was fetched"));
+#endif
       m_env->DeleteGlobalRef(m_exception);
   }
 public:



RE: svn commit: r1664939 - /subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Branko Čibej [mailto:brane@wandisco.com]
> Sent: zondag 8 maart 2015 08:25
> To: Subversion Development
> Subject: Re: svn commit: r1664939 -
> /subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
> 
> On 08.03.2015 02:22, rhuijben@apache.org wrote:
> > Author: rhuijben
> > Date: Sun Mar  8 01:22:15 2015
> > New Revision: 1664939
> >
> > URL: http://svn.apache.org/r1664939
> > Log:
> > Following up on r1664938, add some debug code to make 'Java error leaks'
> > visible in maintainer/test mode.
> >
> > * subversion/bindings/javahl/native/JNIUtil.cpp
> >   (WrappedException): Track whether the exception is fetched. Use DBG
> output
> >     if it isn't.
> >
> > Modified:
> >     subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
> >
> > Modified: subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/nati
> ve/JNIUtil.cpp?rev=1664939&r1=1664938&r2=1664939&view=diff
> >
> ================================================================
> ==============
> > --- subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp (original)
> > +++ subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp Sun Mar
> 8 01:22:15 2015
> > @@ -1079,6 +1079,9 @@ class WrappedException
> >  {
> >    JNIEnv *m_env;
> >    jthrowable m_exception;
> > +#ifdef SVN_DEBUG
> > +  bool m_fetched;
> > +#endif
> >  public:
> >    WrappedException(JNIEnv *env)
> >    {
> > @@ -1092,6 +1095,8 @@ public:
> >
> >      // As adding a reference in exception state fails
> >      m_exception = static_cast<jthrowable>(env-
> >NewGlobalRef(exceptionObj));
> > +
> > +    m_fetched = false;
> >    }
> 
> This breaks non-maintainer-mode.

Thanks.. Fixed in r1664978.

> Also, have you tested how your changes work with the new-style JavaHL
> bits that use the jniwrapper code?

This part was not handled by yesterdays patches... I started by implementing the most common
SVNClient callbacks. But it is easy to apply the same wrapping there, as everything is already in one place there. See r1664984.

	Bert 



Re: svn commit: r1664939 - /subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp

Posted by Branko Čibej <br...@wandisco.com>.
On 08.03.2015 02:22, rhuijben@apache.org wrote:
> Author: rhuijben
> Date: Sun Mar  8 01:22:15 2015
> New Revision: 1664939
>
> URL: http://svn.apache.org/r1664939
> Log:
> Following up on r1664938, add some debug code to make 'Java error leaks'
> visible in maintainer/test mode.
>
> * subversion/bindings/javahl/native/JNIUtil.cpp
>   (WrappedException): Track whether the exception is fetched. Use DBG output
>     if it isn't.
>
> Modified:
>     subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
>
> Modified: subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp?rev=1664939&r1=1664938&r2=1664939&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp (original)
> +++ subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp Sun Mar  8 01:22:15 2015
> @@ -1079,6 +1079,9 @@ class WrappedException
>  {
>    JNIEnv *m_env;
>    jthrowable m_exception;
> +#ifdef SVN_DEBUG
> +  bool m_fetched;
> +#endif
>  public:
>    WrappedException(JNIEnv *env)
>    {
> @@ -1092,6 +1095,8 @@ public:
>  
>      // As adding a reference in exception state fails
>      m_exception = static_cast<jthrowable>(env->NewGlobalRef(exceptionObj));
> +
> +    m_fetched = false;
>    }

This breaks non-maintainer-mode.

Also, have you tested how your changes work with the new-style JavaHL
bits that use the jniwrapper code?

-- Brane