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,