Re: svn commit: r1718167 -/subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c

[Evgeny Kotkov <ev...@visualsvn.com>]

Bert Huijben <be...@qqmail.nl> writes:

> What is the real problem you try to solve here?
>
> Ra apis are not able to support other apis during callbacks or when editors
> are open… This is part of the ra contract as documented in svn_ra.h.
>
> I don’t think we explicitly want to support users who break this contract.

[...]

> Ra local supports a lot of things because it is a very thin layer over the
> lower layers… (I once accidentally found out that you can get very far
> passing bad batons during commit). But the only thing we promise is that it
> supports the ra contract.

This change is not about providing support for cases when someone violates
the RA contract:

  * The caller may not perform any RA operations using @a session before
  * finishing the report, and may not perform any RA operations using
  * @a session from within the editing operations of @a update_editor.

That's because the situation is different when zero-copy code path is being
used.  In that case, the caller can't use the FS API or any other API functions
using FS API from the callbacks, even if they occur on a completely unrelated
object.  This is so because the zero-copy send happens while holding the lock
for the cache.  The cache object is a per-process singleton, so, the callback
is executed while basically holding a global mutex.

We are lucky that, for instance, svnsync doesn't use the reporter in its
implementation, but an arbitrary svnsync-like application or other API users
have a chance of hitting the described problems once they perform an API
call from within the editor callback.  As another example, our diff editor
can perform RA calls using an extra RA session in some cases.  If a call like
that happened from within a zero-copy code path, it would cause problems in
the ra_local case.


Regards,
Evgeny Kotkov

Re: svn commit: r1718167 -/subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c

[Evgeny Kotkov <ev...@visualsvn.com>]

Bert Huijben <be...@qqmail.nl> writes:

> Any ideas how this affects editor v2 (And how it is used in JavaHL for 1.9)?
> Could this hit the limitations?

Ev2 in JavaHL is not affected by this problem.

When JavaHL asks the RA layer to drive the editor, it passes an Ev2-to-Ev1
shim as the editor.  Within this shim, applying delta is handled by the
ev2_apply_textdelta() function, and, due to how it's written, this happens
without any function calls that could access the cache.  Hence, it cannot
hit the limitations of the zero-copy code path.


Regards,
Evgeny Kotkov

RE: svn commit: r1718167 -/subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c

[Bert Huijben <be...@qqmail.nl>]

> -----Original Message-----
> From: Evgeny Kotkov [mailto:evgeny.kotkov@visualsvn.com]
> Sent: maandag 7 december 2015 12:11
> To: Bert Huijben <be...@qqmail.nl>
> Cc: Subversion Development <de...@subversion.apache.org>
> Subject: Re: svn commit: r1718167 -
> /subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
> 
> Bert Huijben <be...@qqmail.nl> writes:
> 
> > What is the real problem you try to solve here?
> >
> > Ra apis are not able to support other apis during callbacks or when
> > editors are open… This is part of the ra contract as documented in svn_ra.h.
> >
> > I don’t think we explicitly want to support users who break this contract.
> 
> [...]
> 
> > Ra local supports a lot of things because it is a very thin layer over
> > the lower layers… (I once accidentally found out that you can get very
> > far passing bad batons during commit). But the only thing we promise
> > is that it supports the ra contract.
> 
> This change is not about providing support for cases when someone violates
> the RA contract:
> 
>   * The caller may not perform any RA operations using @a session before
>   * finishing the report, and may not perform any RA operations using
>   * @a session from within the editing operations of @a update_editor.
> 
> That's because the situation is different when zero-copy code path is being
> used.  In that case, the caller can't use the FS API or any other API functions
> using FS API from the callbacks, even if they occur on a completely unrelated
> object.  This is so because the zero-copy send happens while holding the lock
> for the cache.  The cache object is a per-process singleton, so, the callback is
> executed while basically holding a global mutex.
> 
> We are lucky that, for instance, svnsync doesn't use the reporter in its
> implementation, but an arbitrary svnsync-like application or other API users
> have a chance of hitting the described problems once they perform an API
> call from within the editor callback.  As another example, our diff editor can
> perform RA calls using an extra RA session in some cases.  If a call like that
> happened from within a zero-copy code path, it would cause problems in the
> ra_local case.

Thanks for this explanation. This wasn't clear for me when reading the original patch and the backport proposal. With that explanation I would have added my +1.

Any ideas how this affects editor v2 (And how it is used in JavaHL for 1.9)? Could this hit the limitations?

	Bert