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?
Below is a patch along the lines of what I'm thinking for the
sysv semaphores.
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-14 15:57:58.000000000 -0500
@@ -196,9 +196,16 @@
{
union semun ick;
apr_status_t rv;
+ key_t key;
+
+ if (fname) {
+ key = ftok(fname, 'K');
+ } else {
+ key = IPC_PRIVATE;
+ }
new_mutex->interproc = apr_palloc(new_mutex->pool,
sizeof(*new_mutex->interproc));
- new_mutex->interproc->filedes = semget(IPC_PRIVATE, 1, IPC_CREAT | 0600);
+ new_mutex->interproc->filedes = semget(key, 1, IPC_CREAT | 0600);
if (new_mutex->interproc->filedes < 0) {
rv = errno;