[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Updating exclusively opened files locks working copy

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2006-07-30 12:16:05 CEST

Eric Hanchrow <offby1@blarg.net> writes:

> I know precisely nothing about APR programming, so forgive me if this
> is stupid, but: doesn't this leak a file handle?
> + /* Make sure the file is not exclusively opened by someone else. */
> + if (!adding && entry && entry->schedule != svn_wc_schedule_delete)
> + {
> + apr_file_t* file;
> + SVN_ERR(svn_io_file_open(&file, fb->path, APR_READ, APR_OS_DEFAULT,
> + subpool));
> + }

The file will be closed when the subpool is destroyed and that happens
immediately after this code. Once the file has been closed it might
then get locked and so the problem has not been eliminated but simply
reduced. It's a bit like doing stat() before open() to check whether
the file exists, it's not a good solution since open() still has to
handle errors.

In this case it would be better to improve the error handling that is
going wrong, although the original report is short on details so it's
hard to know what to suggest and log handling was mentioned which can
make things tricky. If it turns out that the patch is the best way to
do things, then it needs a much better comment explaining what is
going on. One further disadvantage of the patch is that the code for
the "solution" is a long way from the code that causes the "problem"
and there is no regression test.

Philip Martin
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jul 30 12:16:34 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.