Kamesh Jayachandran wrote:
> C. Michael Pilato wrote:
>> Kamesh Jayachandran wrote:
>>> C. Michael Pilato wrote:
>>>> Today, I was poking thru the BDB code, and noticed the new public FS
>>>> for merge tracking. As I dug into them a little bit deeper, though, I
>>>> something that didn't seem quite right: base_txn_merge_info() doesn't
>>>> its read of the txn's proplist in a BDB transaction. Was that an
>>>> intentional deviation from the way svn_fs_base__txn_proplist() works
>>>> though the two are super-similar in functionality)?
>>> Yes it is intentional.
>>> If we use 'svn_fs_base__retry_txn' instead of 'svn_fs_base__retry' it
>>> tries to open one more trail and hence aborts.
>>> 'begin_txn' in subversion/libsvn_fs_base/trail.c has the following
>>> /* [*]
>>> If we're already inside a trail operation, abort() -- this is
>>> a coding problem (and will likely hang the repository
>>> anyway). */
>> base_txn_merge_info() is a vtable implementation of a public function,
>> is not a trail-protected function. It should be able to call
>> svn_fs_base__retry_txn() just fine. If you're getting an abort() when
>> that way, it's because you're calling into an FS vtable function from
>> other trail-protected function, which is both unnecessary and a
>> violation of
>> the (admittedly undocumented) libsvn_fs_base API usage rules.
>> If the only intention here was avoiding an abort(), then the code is
>> just wrong.
> It happens because we call 'base_txn_mergeinfo' indirectly.
> Call chain upon commit in the order of call.
> txn_vtable's commit_txn hook(svn_fs_base__commit_txn)
> svn_fs_merge_info__update_index (available in libsvn_fs_util)
> txn_vtable's get_mergeinfo hook(base_txn_mergeinfo)
Okay, that's a violation of the libsvn_fs_base APIs. Here are the rules:
* Use transaction trails where you need them to implement public FS APIs
* Do *NOT* call a public API that uses transaction trails from inside
another transaction trails.
You're violating the first rule because you aren't transaction-protecting
your read of the proplist. But the naive fix will cause a violation of the
But I think that latter situation is the real problem. Why in the world do
we have calls into an FS plugin from the FS_util library?! The util library
may be called *by* the FS plugins and FS loader, but shouldn't be calling
*into* either of those things.
C. Michael Pilato <email@example.com>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on Mon Apr 2 17:27:32 2007