On Mon, Jun 01, 2009 at 01:08:04PM -0400, C. Michael Pilato wrote:
> Paul Burba wrote:
> > Avoid temp file name collisions when committing with ra_serf/ra_neon.
> >
> > * subversion/libsvn_ra_neon/commit.c
> > (commit_apply_txdelta): Use svn_io_open_uniquely_named directly instead of
> > svn_io_open_unique_file3. Avoid temp file naming collisions by basing the
> > temp file name on BASE_CHECKSUM, or for new files which have no checksum,
> > on FILE->RSRC->LOCAL_PATH.
> >
> > * subversion/libsvn_ra_serf/commit.c
> > (apply_textdelta): Same as above, but use CTX->NAME instead of
> > FILE->RSRC->LOCAL_PATH.
>
> Please just pick one uniqueness mechanism instead of two. The base_checksum
> thing seems like it would just fall over when committing 10,000 empty files,
> right?
>
> The "edit path" of the file would be necessarily unique, though, and a
> checksum thereof almost as unique, but also less predictable. I'd go that
> route.
I didn't have much time yet to look at this.
But the first question I have is why we don't use apr_file_mktemp()
to open temporary files which aren't meant to be used by users directly?
It certainly makes sense to have numbered tempfiles for commit messages
and such, but for files we create and then rename it's a performance hit
as Paul explained.
apr_file_mktemp() exists in APR 0.9.0 and upwards and is supposed to work
on Win32 and POSIX. It quickly creates a unique random name for the file
and opens it for us, too.
It looks like the decision to implement our own function for this
purpose dates back from before Subversion was self-hosting:
$ svn blame subversion/include/svn_io.h | grep "Claim of Historical Inevitability"
1 svn * Claim of Historical Inevitability: this function was written
The full text of part of svn_io_open_uniquely_named()'s docstring is:
* Claim of Historical Inevitability: this function was written
* because
*
* - tmpnam() is not thread-safe.
* - tempname() tries standard system tmp areas first.
Are these concerns still true for apr_file_mktemp()?
Skimming its code I cannot find anything that looks like it wasn't
thread-safe, but I may well be missing something.
Thanks,
Stefan
Received on 2009-06-01 19:47:41 CEST