Oops, forgot the list.
Ben Collins-Sussman <sussman@collab.net> writes:
> Notice that instead of a delete_tmp_file() callback, open_tmp_file
> simply adds the APR_DELONCLOSE flag. So calling apr_file_close(fp) is
> all ra_dav needs to do. (open_tmp_file also calls the thread-safe
> svn_io_open_unique_file() under the hood, so ra_dav doesn't have to
> anymore.)
If apr_file_open() is passed APR_DELONCLOSE it calls unlink() on the
newly opened file. This deletes the name from the filesystem. The OS
will then delete the file when all the file descriptors are closed.
>
> So now I discover ickiness... apparently in both the commit and update
> cases, ra_dav needs to hand a vanilla "FILE *" version of the tmpfile
> to neon; an apr_file_t won't do. So ra_dav closes the apr_file_t,
> then fopen()/fclose()'s the file for neon.
>
> Being ignorant, I say, "this is bad, because of delete-on-close". So
> I move the apr_file_close() -after- all the fopen/fclose business (for
> example, I do this in commit.c:commit_stream_close).
fopen() will fail because the name has gone.
>
> Still though -- I get a segfault when committing, specifically in
> commit_stream_close, because fopen() tries to reopen the tmpfile, and
> it's already gone somehow.
Yup!
Even if you don't use APR_DELONCLOSE reopening with the filename is a
*really* bad idea, you are introducing a race. The file might get
renamed between the two opens, and another file with the original name
may get created.
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:41 2006