You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Vlad Georgescu <vg...@gmail.com> on 2008/08/18 22:08:57 UTC
Pool usage problems in {base,fs}_node_origin_rev
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.
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.
--
Vlad
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Pool usage problems in {base,fs}_node_origin_rev
Posted by Vlad Georgescu <vg...@gmail.com>.
C. Michael Pilato wrote:
> Vlad Georgescu wrote:
>> When running the tests I get all sorts of failures, mostly in the merge tests.
>> The cause is incorrect pool usage.
>
> Vlad, can you sanity-check r32545, please? Thanks.
>
The crashes are gone, thanks.
--
Vlad
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Pool usage problems in {base,fs}_node_origin_rev
Posted by "C. Michael Pilato" <cm...@collab.net>.
Vlad Georgescu wrote:
> When running the tests I get all sorts of failures, mostly in the merge tests.
> The cause is incorrect pool usage.
Vlad, can you sanity-check r32545, please? Thanks.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Pool usage problems in {base,fs}_node_origin_rev
Posted by "C. Michael Pilato" <cm...@collab.net>.
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 <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand