[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] JavaHL: Explicitly pass jobject jthis when processing dispose() call rather than stashing a reference in the SVNBase class where it can be missused later

From: Vladimir Berezniker <vmpn_at_hitechman.com>
Date: Fri, 25 May 2012 16:47:41 -0400

Object hold their baton. Editor object also holds the editor driver (if I
am understanding the terminology correctly).

In detail:

Editor holds reference to the editor and its baton plus parent/child object
references:

  const svn_delta_editor_t * m_deltaEditor;
    void * m_editBaton;
    SVNRa & m_parentRaSession;
    CommitCallback * m_commitCallback;

    // Because of restriction #3 from svn_delta.h we only need to keep
track of one directory
    SVNDirectory * m_rootDirectory;

    //But we do have to keep track of all files as delta's can be sent at
the end
    std::set<SVNFile *> m_svnFiles;

Directory holds its baton plus parent/child object references:

    void * m_dirBaton;
    SVNEditor & m_parentEditor;
    IDirectoryParent & m_parent;
    SVNDirectory * m_childDirectory;

Vladimir

On Fri, May 25, 2012 at 4:30 PM, Hyrum K Wright
<hyrum.wright_at_wandisco.com>wrote:

> Are you talking about maintaining the directory baton in the context
> of something like an editor drive?
>
> -Hyrum
>
> On Fri, May 25, 2012 at 2:29 PM, Vladimir Berezniker <vmpn_at_hitechman.com>
> wrote:
> > The reason they are long lived is because they are reusable. E.g.
> directory
> > can be used for any number of files/directories within it. However,
> > regarding not needing to keep track of objects I think I see where you
> > are coming from. At the moment the parent and child objects are tied
> because
> > I allocated child pools from parent pools therefore their order of
> cleanup
> > is important. The reason for that design was that I could free up child
> > resources when the parent was disposed even if the children were not.
> But of
> > the top of my head I cannot see why it has to be this way. For
> example child
> > object pools can be allocated from the global JNI pool instead
> > thus eliminating the need to track children. Code will be simpler but
> the
> > JavaHL users won't be as protected from themselves.
> >
> > I need to dig through the code and think about it some more,
> >
> > Vladimir
> >
> >
> > On Fri, May 25, 2012 at 2:36 PM, Hyrum K Wright <
> hyrum.wright_at_wandisco.com>
> > wrote:
> >>
> >> +1 to commit to trunk. Please include "Approved by: hwright" in the
> >> log message.
> >>
> >> Out of curiosity, why do directory objects need to live across method
> >> invocations with the RA interface? I would have thought that even if
> >> this is true, it won't matter, since those objects would be pure Java
> >> objects handled by the JVM.
> >>
> >> -Hyrum
> >>
> >> On Fri, May 25, 2012 at 12:35 PM, Vladimir Berezniker
> >> <vmpn_at_hitechman.com> wrote:
> >> > 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
> >>
> >>
> >>
> >> --
> >>
> >> uberSVN: Apache Subversion Made Easy
> >> http://www.uberSVN.com/
> >
> >
>
>
>
> --
>
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com/
>
Received on 2012-05-25 22:48:16 CEST

This is an archived mail posted to the Subversion Dev mailing list.