Re: Pool usage problems in {base,fs}_node_origin_rev

Vlad Georgescu wrote:
> When running the tests I get all sorts of failures, mostly in the merge tests.
> The cause is incorrect pool usage.
>
> In libsvn_fs_base/tree.c, base_node_origin_rev(), line 4753:
>
> /* Walk the predecessor links back to origin. */
> SVN_ERR(base_node_id(&pred_id, curroot, lastpath->data, pool));
> while (1)
> {
> struct txn_pred_id_args pid_args;
> svn_pool_clear(subpool);
> pid_args.id = pred_id;
> pid_args.pool = subpool;
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_pred_id,
> &pid_args, subpool));
> if (! pid_args.pred_id)
> break;
> pred_id = pid_args.pred_id;
> }
>
> Notice that in all iterations except the first, pred_id will be used after being
> freed.

Yep. Good catch.

> The equivalent code in libsvn_fs_fs/tree.c, fs_node_origin_rev(), line 3071:
>
> /* Walk the predecessor links back to origin. */
> SVN_ERR(fs_node_id(&pred_id, curroot, lastpath->data, predidpool));
> while (pred_id)
> {
> svn_pool_clear(subpool);
> SVN_ERR(svn_fs_fs__dag_get_node(&node, fs, pred_id, subpool));
> svn_pool_clear(predidpool);
> SVN_ERR(svn_fs_fs__dag_get_predecessor_id(&pred_id, node,
> predidpool));
> }
>
> This looks correct, except that svn_fs_fs__dag_get_predecessor_id() sometimes
> returns a cached value obtained directly from node, thus causing the same sort
> of problem.

I hate hate HATE HAAAAAAAAAAAATE functions that don't honor the 'pool'
parameter. This is a deep, burning, passionate hatred of the sort that
disfigures a man's countenance.

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand