On Thu, Jan 14, 2010 at 03:47:03PM -0500, Bob Rossi wrote:
> Hi,
>
> I've noticed in the global mutex code, what I consider to be a problem.
> I'm trying to get a global mutex, that multiple process's can use to
> ensure 1 access at a time.
>
> For APR_LOCK_SYSVSEM, in proc_mutex_sysv_create, semget is called with
> IPC_PRIVATE, instead of using ftok to convert the filename passed in
> to a key. Please correct me if I'm wrong, but this forces the lock to
> only be useful in the same process, and with children of that process.
> This isn't useful for me. Shouldn't ftok be called if the filename is
> non NULL?
>
> For APR_LOCK_FCNTL I notice a similar problem. In
> proc_mutex_fcntl_create, after the file is created, it's unlinked. By
> calling unlink, another process can't access it, to try to lock it. Was
> does this function behave this way?
> Also, for this case, if the file already exists on disk, you can't
> open it, you get an error. Shouldn't it allow opening an already
> existing file?
>
> Now, on windows, using APR_LOCK_DEFAULT, it calls into
> apr_proc_mutex_create. In this case, it uses CreateMutex but DOES
> convert the filename into a key to pass into CreateMutex. This allows
> multiple process's to lock on a key properly.
>
> So, I've described a few cases. Windows works fine for me, UNIX does
> not with both sysv and fcntl. Is this designed this way, or is it a
> mistake? I would suggest mainly that the sysv semget function
> use IPCPRIVATE when the fname is NULL and ftok when it's non NULL.
> Thoughts?
Here's the patch that makes file locking useful between process's
using fcntl.
Thanks,
Bob Rossi
--- apr/locks/unix/proc_mutex.c 2007-11-19 18:17:50.000000000 -0500
+++ apr.new/locks/unix/proc_mutex.c 2010-01-15 06:41:01.000000000 -0500
@@ -519,7 +519,7 @@
if (fname) {
new_mutex->fname = apr_pstrdup(new_mutex->pool, fname);
rv = apr_file_open(&new_mutex->interproc, new_mutex->fname,
- APR_CREATE | APR_WRITE | APR_EXCL,
+ APR_CREATE | APR_WRITE,
APR_UREAD | APR_UWRITE | APR_GREAD | APR_WREAD,
new_mutex->pool);
}
@@ -535,7 +535,6 @@
}
new_mutex->curr_locked = 0;
- unlink(new_mutex->fname);
apr_pool_cleanup_register(new_mutex->pool,
(void*)new_mutex,
apr_proc_mutex_cleanup,
