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

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

From: Philip Martin <philip.martin_at_wandisco.com>
Date: Wed, 31 Oct 2012 13:15:52 +0000

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?

>> }
>>
>> - if (apr_err)
>> - return unlock(&new_ns->mutex,
>> - svn_error_wrap_apr(apr_err,
>> - _("Can't get shared memory for named atomics")));
>> + /* Now, map it into memory.
>> + */
>
> Need to do something with err, at present it leaks. The mmap call
> probably doesn't make sense if err is set.
>
>> + apr_err = apr_mmap_create(&mmap, file, 0, sizeof(*new_ns->data),
>> + APR_MMAP_READ | APR_MMAP_WRITE , result_pool);
>> + if (!apr_err)
>> + new_ns->data = mmap->mm;
>>
>> + svn_pool_destroy(sub_pool);
>
> --
> Certified & Supported Apache Subversion Downloads:
> http://www.wandisco.com/subversion/download
>

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Received on 2012-10-31 14:16:29 CET

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