Date: Sun, 4 May 2014 12:50:01 GMT From: Joris Giovannangeli <joris@giovannangeli.fr> To: freebsd-standards@FreeBSD.org Subject: Re: standards/189353: POSIX sem_unlink does not actually unlink the semaphore in the process context Message-ID: <201405041250.s44Co15R080863@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR standards/189353; it has been noted by GNATS.
From: Joris Giovannangeli <joris@giovannangeli.fr>
To: Konstantin Belousov <kostikbel@gmail.com>
Cc: freebsd-gnats-submit@FreeBSD.org
Subject: Re: standards/189353: POSIX sem_unlink does not actually unlink the
semaphore in the process context
Date: Sun, 04 May 2014 14:46:23 +0200
This is a multi-part message in MIME format.
--------------030807090400060606050102
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
On 04/05/2014 14:37, Konstantin Belousov wrote:
> diff --git a/lib/libc/gen/sem_new.c b/lib/libc/gen/sem_new.c index
> 9a2ab27..8e4a91d 100644 --- a/lib/libc/gen/sem_new.c +++
> b/lib/libc/gen/sem_new.c @@ -161,7 +161,7 @@ _sem_open(const char
> *name, int flags, ...)
>
> _pthread_mutex_lock(&sem_llock); LIST_FOREACH(ni, &sem_list, next)
> { - if (strcmp(name, ni->name) == 0) { + if (ni->name != NULL &&
> strcmp(name, ni->name) == 0) { if ((flags & (O_CREAT|O_EXCL)) ==
> (O_CREAT|O_EXCL)) { _pthread_mutex_unlock(&sem_llock); errno =
> EEXIST; @@ -287,20 +287,32 @@ _sem_close(sem_t *sem) int
> _sem_unlink(const char *name) { + struct sem_nameinfo *ni; char
> path[PATH_MAX]; + int error;
>
> if (name[0] != '/') { errno = ENOENT; return -1; } name++; -
> strcpy(path, SEM_PREFIX); if (strlcat(path, name, sizeof(path)) >=
> sizeof(path)) { errno = ENAMETOOLONG; return (-1); } - return
> unlink(path); + + _pthread_once(&once, sem_module_init); +
> _pthread_mutex_lock(&sem_llock); + LIST_FOREACH(ni, &sem_list,
> next) { + if (ni->name != NULL && strcmp(name, ni->name) == 0) { +
> ni->name = NULL; + break; + } + } + error = unlink(path); +
> _pthread_mutex_unlock(&sem_llock); + return (error); }
>
> int
>
I've found this issue while implementing semaphores for dragonflyBSD,
and at first i came with the same solution than yours. But it won't
work when you do the same in two processes : unlink will only remove
the caching on the process calling unlink, not all of them. See
attached test case.
It looks like glibc is using inodes numbers and dev number for that,
which, while not being strictly correct (inodes can be reused), seems
to work fine on linux.
For dragonfly, i'm currently trying to keep filedescriptors opened in
the nameinfo structure. This adds quite a bit of overhead, but this
way you can check link count with fstat during a sem_open to see if
the semaphore file still exist, and return the cached mapping only if
st_nlink > 0.
This solution at least should be strictly correct, but the inode
solution could be fine in practice.
Regards,
joris
--------------030807090400060606050102
Content-Type: text/x-csrc;
name="sem_test.c"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="sem_test.c"
#include <fcntl.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <sys/types.h>
#include <semaphore.h>
#include <stdio.h>
#include <unistd.h>
#include <err.h>
#include <errno.h>
int main() {
pid_t pid;
sem_t *sem1, *sem2;
int error, saved_errno, status;
int v1, v2;
int pd[2];
char er[1];
char ok[1];
char buf[1];
size_t size;
*er = 'X';
*ok = 'O';
if (pipe(pd) == -1)
err(1, "Cannot create sync pipe");
sem1 = sem_open("/test-sem", O_CREAT | O_EXCL, S_IRWXU, 1);
if (sem1 == SEM_FAILED)
err(1,"Cannot create test /test-sem semaphore");
char c = getchar();
pid = fork();
if ( pid == -1)
err(1, "Unable to fork test process");
if (pid == 0) {
close(pd[0]);
error = sem_unlink("/test-sem");
if (error) {
perror("Cannot unlink semaphore /test-sem");
goto error;
}
sem2 = sem_open("/test-sem", O_CREAT | O_EXCL, S_IRWXU, 2);
if (sem2 == SEM_FAILED) {
perror("Cannot re-create semaphore /test-sem");
goto error;
}
write(pd[1], ok, 1);
error = wait(&status);
if (error && errno == EINTR)
goto error;
return(status);
error:
write(pd[1], er, 1);
close(pd[1]);
error = wait(&status);
if (error && errno == EINTR)
goto error;
errx(1, "TEST UNRESOLVED");
return(1);
} else {
close(pd[1]);
size = read(pd[0], buf, 1);
if (size != 1)
err(2, "process 2 cannot wait for process 1");
if (*buf == *er)
errx(2, "Test error");
else {
sem2 = sem_open("/test-sem", 0);
if (sem2 == SEM_FAILED)
err(2, "process 2 failed reopeing semaphore");
// sem_unlink("/test-sem");
sem_getvalue(sem1, &v1);
sem_getvalue(sem2, &v2);
printf("v1 = %d and v2 = %d\n", v1, v2);
if (v1 == v2) {
errx(2, "*** TEST FAILED. NOT POSIX ***");
} else {
errx(0, "*** TEST SUCCESS. POSIX CONFORMANT ***");
}
}
}
}
--------------030807090400060606050102--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201405041250.s44Co15R080863>