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: [email protected] Sent: zondag 6 december 2015 13:02 To: [email protected] 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
