Re: svn commit: r1404112 - in /subversion/trunk/subversion: include/private/svn_named_atomic.h libsvn_fs_fs/fs_fs.c libsvn_subr/named_atomic.c tests/cmdline/svnadmin_tests.py tests/libsvn_subr/named_atomic-test.c

On Wed, Oct 31, 2012 at 2:15 PM, Philip Martin
<philip.martin_at_wandisco.com>wrote:

> Philip Martin <philip.martin_at_wandisco.com> writes:
>
> > stefan2_at_apache.org writes:
> >
> >> Author: stefan2
> >> Date: Wed Oct 31 12:27:29 2012
> >> New Revision: 1404112
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1404112&view=rev
> >> Log:
> >> Change the way we implement shared memory setup for our named atomics.
> >> Instead of using APR's SHM API, we create a persistent file and simply
> >> mmap it.
> >
> >> +/* Make sure the revprop_namespace member in FS is set. */
> >> +static svn_error_t *
> >> +cleanup_revprop_namespace(svn_fs_t *fs)
> >> +{
> >> + const char* name = svn_dirent_join(fs->path,
> >
> > const char *name
> >
> >> + err = svn_io_check_path(shm_name, &kind, sub_pool);
> >> + if (!err && kind != svn_node_file)
> >> {
> >> - apr_err = apr_shm_attach(&shared_mem, shm_name, result_pool);
> >> - if (!apr_err)
> >> - {
> >> - new_ns->data = apr_shm_baseaddr_get(shared_mem);
> >> - break;
> >> - }
> >> -
> >> - /* First race: failed to attach but another process could
> create. */
> >> -
> >> - apr_err = apr_shm_create(&shared_mem,
> >> - sizeof(*new_ns->data),
> >> - shm_name,
> >> - result_pool);
> >> - if (!apr_err)
> >> + err = svn_io_file_open(&file, shm_name,
> >> + APR_READ | APR_WRITE | APR_CREATE,
> >> + APR_OS_DEFAULT,
> >> + result_pool);
> >> + if (!err)
> >> {
> >> - new_ns->data = apr_shm_baseaddr_get(shared_mem);
> >> -
> >> - /* Zero all counters, values and names. */
> >> - memset(new_ns->data, 0, sizeof(*new_ns->data));
> >> - break;
> >> + /* Zero all counters, values and names.
> >> + */
> >> + struct shared_data_t initial_data;
> >> + memset(&initial_data, 0, sizeof(initial_data));
> >> + err = svn_io_file_write_full(file, &initial_data,
> >> + sizeof(initial_data), NULL,
> >> + sub_pool);
> >> }
> >> -
> >> - /* Second race: failed to create but another process could
> delete. */
> >> + }
> >> + else
> >> + {
> >> + err = svn_io_file_open(&file, shm_name,
> >> + APR_READ | APR_WRITE, APR_OS_DEFAULT,
> >> + result_pool);
>
> Suppose the process that first created the file got interrupted between
> the svn_io_file_open and svn_io_file_write_full calls. The file is
> incomplete but this process is going to assume it is correct and use it.
> Will that work? Perhaps we should stat() the file to check the size?
>

Thanks for the review(s)!

As of r1404138, all issues should have been addressed.
Let's see what the build bots have to say about that ;)

-- Stefan^2.

-- 
Certified & Supported Apache Subversion Downloads:
*
http://www.wandisco.com/subversion/download
*