subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bert Huijben <b...@qqmail.nl>
Subject RE: svn commit: r1718167 -/subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
Date Mon, 07 Dec 2015 00:22:59 GMT
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.

That some things may work, doesn’t make them supported… which brings me back to the original
question. 

I’m unable to review changes like this… Without further explanation I don’t see any
reason for this change… and certainly no reason for directly backporting this.

There are hundreds of apis that segfault on passing NULL, while some others may check for
them as side effect of how they are implemented. That doesn’t necessarily make that part
of their api contract.

That an api may be abused doesn’t require us to support abusing it.

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.

Bert

Sent from Outlook Mail for Windows 10 phone


From: kotkov@apache.org
Sent: zondag 6 december 2015 13:02
To: commits@subversion.apache.org
Subject: svn commit: r1718167 -/subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c

Author: kotkov
Date: Sun Dec  6 12:02:00 2015
New Revision: 1718167

URL: http://svn.apache.org/viewvc?rev=1718167&view=rev
Log:
Disable zero-copy code path for ra_local update reporters.

The known and documented limitation of svn_repos_begin_report3() is that
with zero-copy enabled, the delta editor callbacks cannot access FSFS
or use Subversion caches directly.  This limitation comes from the fact
that sending delta using the zero-copy code path happens from within a cache
access wrapper — that is, while holding the lock.  If a particular delta
consumer happens to access or invalidate the cache, bad things could happen,
spanning from UB due to accessing a dangling pointer to a deadlock caused by
an attempt to take a non-recursive (blocking) lock, that has already been
taken by the same thread.

Within ra_local, we cannot be sure that arbitrary callers of our public
API, namely, svn_ra_do_update3(), svn_ra_do_switch3() or svn_ra_do_status2(),
are aware of this limitation and pass-in the delta editor that doesn't access
FSFS or caches — because everything happens locally and all operations that
use the FS layer have a chance of using the cache.

* subversion/libsvn_ra_local/ra_plugin.c
  (make_reporter): Pass 0 as zero_copy_limit when creating the update
   reporter.

Modified:
    subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c

Modified: subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c?rev=1718167&r1=1718166&r2=1718167&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c (original)
+++ subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c Sun Dec  6 12:02:00 2015
@@ -360,8 +360,13 @@ make_reporter(svn_ra_session_t *session,
                                   edit_baton,
                                   NULL,
                                   NULL,
-                                  1024 * 1024,  /* process-local transfers
-                                                   should be fast */
+                                  0, /* Disable zero-copy codepath, because
+                                        RA API users are unaware about the
+                                        zero-copy code path limitation (do
+                                        not access FSFS data structures
+                                        and, hence, caches).  See notes
+                                        to svn_repos_begin_report3() for
+                                        additional details. */
                                   result_pool));
 
   /* Wrap the report baton given us by the repos layer with our own




Mime
View raw message