Update.
authorUlrich Drepper <drepper@redhat.com>
Sat, 17 May 2003 20:53:32 +0000 (20:53 +0000)
committerUlrich Drepper <drepper@redhat.com>
Sat, 17 May 2003 20:53:32 +0000 (20:53 +0000)
2003-05-17  Ulrich Drepper  <drepper@redhat.com>

* sem_open.c: Fix one endless loop.  Implement correct semantics
wrt opening the same semaphore more then once.
* sem_close.c: Adjust for sem_open change.
* semaphoreP.h: Include <semaphore.h>.  Define struct inuse_sem.
Declare __sem_mappings, __sem_mappings_lock, __sem_search.
* Makefile (tests): Add tst-sem7.
* tst-sem7.c: New file.

nptl/ChangeLog
nptl/Makefile
nptl/sem_open.c
nptl/semaphoreP.h
nptl/tst-sem7.c [new file with mode: 0644]

index 0813aaf..0f4d085 100644 (file)
@@ -1,3 +1,13 @@
+2003-05-17  Ulrich Drepper  <drepper@redhat.com>
+
+       * sem_open.c: Fix one endless loop.  Implement correct semantics
+       wrt opening the same semaphore more then once.
+       * sem_close.c: Adjust for sem_open change.
+       * semaphoreP.h: Include <semaphore.h>.  Define struct inuse_sem.
+       Declare __sem_mappings, __sem_mappings_lock, __sem_search.
+       * Makefile (tests): Add tst-sem7.
+       * tst-sem7.c: New file.
+
 2003-05-16  Roland McGrath  <roland@redhat.com>
 
        * sysdeps/unix/sysv/linux/register-atfork.c (libc_freeres_fn): Fix
index a166dda..31faa7c 100644 (file)
@@ -149,7 +149,7 @@ tests = tst-attr1 tst-attr2 \
        tst-rwlock11 \
        tst-once1 tst-once2 tst-once3 tst-once4 \
        tst-key1 tst-key2 tst-key3 tst-key4 \
-       tst-sem1 tst-sem2 tst-sem3 tst-sem4 tst-sem5 tst-sem6 \
+       tst-sem1 tst-sem2 tst-sem3 tst-sem4 tst-sem5 tst-sem6 tst-sem7 \
        tst-barrier1 tst-barrier2 tst-barrier3 \
        tst-align \
        tst-basic1 tst-basic2 tst-basic3 tst-basic4 tst-basic5 tst-basic6 \
index 2bfea63..374c7d8 100644 (file)
@@ -22,6 +22,7 @@
 #include <mntent.h>
 #include <paths.h>
 #include <pthread.h>
+#include <search.h>
 #include <semaphore.h>
 #include <stdarg.h>
 #include <stdio.h>
@@ -122,11 +123,105 @@ __where_is_shmfs (void)
 }
 
 
+/* Comparison function for search of existing mapping.  */
+int
+attribute_hidden
+__sem_search (const void *a, const void *b)
+{
+  const struct inuse_sem *as = (const struct inuse_sem *) a;
+  const struct inuse_sem *bs = (const struct inuse_sem *) b;
+
+  if (as->ino != bs->ino)
+    /* Cannot return the difference the type is larger than int.  */
+    return as->ino < bs->ino ? -1 : (as->ino == bs->ino ? 0 : 1);
+
+  if (as->dev != bs->dev)
+    /* Cannot return the difference the type is larger than int.  */
+    return as->dev < bs->dev ? -1 : (as->dev == bs->dev ? 0 : 1);
+
+  return strcmp (as->name, bs->name);
+}
+
+
+/* The search tree for existing mappings.  */
+void *__sem_mappings attribute_hidden;
+
+/* Lock to protect the search tree.  */
+lll_lock_t __sem_mappings_lock = LLL_LOCK_INITIALIZER;
+
+
+/* Search for existing mapping and if possible add the one provided.  */
+static sem_t *
+check_add_mapping (const char *name, size_t namelen, int fd, sem_t *existing)
+{
+  sem_t *result = SEM_FAILED;
+
+  /* Get the information about the file.  */
+  struct stat64 st;
+  if (__fxstat64 (_STAT_VER, fd, &st) == 0)
+    {
+      /* Get the lock.  */
+      lll_lock (__sem_mappings_lock);
+
+      /* Search for an existing mapping given the information we have.  */
+      struct inuse_sem *fake;
+      fake = (struct inuse_sem *) alloca (sizeof (*fake) + namelen);
+      memcpy (fake->name, name, namelen);
+      fake->dev = st.st_dev;
+      fake->ino = st.st_ino;
+
+      struct inuse_sem **foundp = tfind (fake, &__sem_mappings, __sem_search);
+      if (foundp != NULL)
+       {
+         /* There is already a mapping.  Use it.  */
+         result = (*foundp)->sem;
+         ++(*foundp)->refcnt;
+       }
+      else if (existing != SEM_FAILED)
+       {
+         /* We haven't found a mapping but the caller has a mapping.
+            Install it.  */
+         struct inuse_sem *newp;
+
+         newp = (struct inuse_sem *) malloc (sizeof (*newp) + namelen);
+         if (newp != NULL)
+           {
+             newp->dev = st.st_dev;
+             newp->ino = st.st_ino;
+             newp->refcnt = 1;
+             newp->sem = existing;
+             memcpy (newp->name, name, namelen);
+
+             /* Insert the new value.  */
+             if (tsearch (newp, &__sem_mappings, __sem_search) != NULL)
+               /* Successful.  */
+               result = existing;
+             else
+               /* Something went wrong while inserting the new
+                  value.  We fail completely.  */
+               free (newp);
+           }
+       }
+
+      /* Release the lock.  */
+      lll_unlock (__sem_mappings_lock);
+    }
+
+  if (result != existing && existing != SEM_FAILED)
+    {
+      /* Do not disturb errno.  */
+      INTERNAL_SYSCALL_DECL (err);
+      INTERNAL_SYSCALL (munmap, err, 2, existing, sizeof (sem_t));
+    }
+
+  return result;
+}
+
+
 sem_t *
 sem_open (const char *name, int oflag, ...)
 {
   char *finalname;
-  size_t namelen;
   sem_t *result = SEM_FAILED;
   int fd;
 
@@ -150,12 +245,12 @@ sem_open (const char *name, int oflag, ...)
       __set_errno (EINVAL);
       return SEM_FAILED;
     }
-  namelen = strlen (name);
+  size_t namelen = strlen (name) + 1;
 
   /* Create the name of the final file.  */
-  finalname = (char *) alloca (mountpoint.dirlen + namelen + 1);
+  finalname = (char *) alloca (mountpoint.dirlen + namelen);
   __mempcpy (__mempcpy (finalname, mountpoint.dir, mountpoint.dirlen),
-            name, namelen + 1);
+            name, namelen);
 
   /* If the semaphore object has to exist simply open it.  */
   if ((oflag & O_CREAT) == 0 || (oflag & O_EXCL) == 0)
@@ -167,15 +262,22 @@ sem_open (const char *name, int oflag, ...)
       if (fd == -1)
        {
          /* If we are supposed to create the file try this next.  */
-         if ((oflag & O_CREAT) != 0)
+         if ((oflag & O_CREAT) != 0 && errno == ENOENT)
            goto try_create;
 
          /* Return.  errno is already set.  */
        }
       else
-       /* Map the sem_t structure from the file.  */
-       result = (sem_t *) mmap (NULL, sizeof (sem_t), PROT_READ | PROT_WRITE,
-                                MAP_SHARED, fd, 0);
+       {
+         /* Check whether we already have this semaphore mapped.  */
+         result = check_add_mapping (name, namelen, fd, SEM_FAILED);
+
+         /* Map the sem_t structure from the file.  */
+         if (result == SEM_FAILED)
+           result = (sem_t *) mmap (NULL, sizeof (sem_t),
+                                    PROT_READ | PROT_WRITE, MAP_SHARED,
+                                    fd, 0);
+       }
     }
   else
     {
@@ -200,14 +302,6 @@ sem_open (const char *name, int oflag, ...)
          return SEM_FAILED;
        }
 
-      tmpfname = (char *) alloca (mountpoint.dirlen + 6 + 1);
-      strcpy (__mempcpy (tmpfname, mountpoint.dir, mountpoint.dirlen),
-             "XXXXXX");
-
-      fd = mkstemp (tmpfname);
-      if (fd == -1)
-       return SEM_FAILED;
-
       /* Create the initial file content.  */
       sem_t initsem;
 
@@ -218,10 +312,44 @@ sem_open (const char *name, int oflag, ...)
       memset ((char *) &initsem + sizeof (struct sem), '\0',
              sizeof (sem_t) - sizeof (struct sem));
 
+      tmpfname = (char *) alloca (mountpoint.dirlen + 6 + 1);
+      char *xxxxxx = __mempcpy (tmpfname, mountpoint.dir, mountpoint.dirlen);
+
+      int retries = 0;
+#define NRETRIES 50
+      while (1)
+       {
+         /* Add the suffix for mktemp.  */
+         strcpy (xxxxxx, "XXXXXX");
+
+         /* We really want to use mktemp here.  We cannot use mkstemp
+            since the file must be opened with a specific mode.  The
+            mode cannot later be set since then we cannot apply the
+            file create mask.  */
+         if (mktemp (tmpfname) == NULL)
+           return SEM_FAILED;
+
+         /* Open the file.  Make sure we do not overwrite anything.  */
+         fd = __libc_open (tmpfname, O_RDWR | O_CREAT | O_EXCL, mode);
+         if (fd == -1)
+           {
+             if (errno == EEXIST)
+               {
+                 if (++retries < NRETRIES)
+                   continue;
+
+                 __set_errno (EAGAIN);
+               }
+
+             return SEM_FAILED;
+           }
+
+         /* We got a file.  */
+         break;
+       }
+
       if (TEMP_FAILURE_RETRY (__libc_write (fd, &initsem, sizeof (sem_t)))
          == sizeof (sem_t)
-         /* Adjust the permission.  */
-         && fchmod (fd, mode) == 0
          /* Map the sem_t structure from the file.  */
          && (result = (sem_t *) mmap (NULL, sizeof (sem_t),
                                       PROT_READ | PROT_WRITE, MAP_SHARED,
@@ -249,6 +377,11 @@ sem_open (const char *name, int oflag, ...)
                  goto try_again;
                }
            }
+         else
+           /* Insert the mapping into the search tree.  This also
+              determines whether another thread sneaked by and already
+              added such a mapping despite the fact that we created it.  */
+           result = check_add_mapping (name, namelen, fd, result);
        }
 
       /* Now remove the temporary name.  This should never fail.  If
@@ -262,7 +395,11 @@ sem_open (const char *name, int oflag, ...)
 
   /* We don't need the file descriptor anymore.  */
   if (fd != -1)
-    (void) __libc_close (fd);
+    {
+      /* Do not disturb errno.  */
+      INTERNAL_SYSCALL_DECL (err);
+      INTERNAL_SYSCALL (close, err, 1, fd);
+    }
 
   return result;
 }
index 0d3f6e5..d14ea92 100644 (file)
@@ -1,4 +1,4 @@
-/* Copyright (C) 2002 Free Software Foundation, Inc.
+/* Copyright (C) 2002, 2003 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
 
    Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
    02111-1307 USA.  */
 
+#include <semaphore.h>
 #include "pthreadP.h"
 
+
 /* Mount point of the shared memory filesystem.  */
 struct mountpoint_info
 {
@@ -26,16 +28,35 @@ struct mountpoint_info
   size_t dirlen;
 };
 
+/* Keeping track of currently used mappings.  */
+struct inuse_sem
+{
+  dev_t dev;
+  ino_t ino;
+  int refcnt;
+  sem_t *sem;
+  char name[0];
+};
+
 
 /* Variables used in multiple interfaces.  */
 extern struct mountpoint_info mountpoint attribute_hidden;
 
 extern pthread_once_t __namedsem_once attribute_hidden;
 
+/* The search tree for existing mappings.  */
+extern void *__sem_mappings attribute_hidden;
+
+/* Lock to protect the search tree.  */
+extern lll_lock_t __sem_mappings_lock;
+
 
 /* Initializer for mountpoint.  */
 extern void __where_is_shmfs (void) attribute_hidden;
 
+/* Comparison function for search in tree with existing mappings.  */
+extern int __sem_search (const void *a, const void *b) attribute_hidden;
+
 
 /* Prototypes of functions with multiple interfaces.  */
 extern int __new_sem_init (sem_t *sem, int pshared, unsigned int value);
diff --git a/nptl/tst-sem7.c b/nptl/tst-sem7.c
new file mode 100644 (file)
index 0000000..a85c73e
--- /dev/null
@@ -0,0 +1,109 @@
+/* Copyright (C) 2003 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <semaphore.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+
+static void
+remove_sem (int status, void *arg)
+{
+  sem_unlink (arg);
+}
+
+
+int
+main (void)
+{
+  sem_t *s;
+  sem_t *s2;
+  sem_t *s3;
+
+  s = sem_open ("/glibc-tst-sem7", O_CREAT, 0600, 1);
+  if (s == SEM_FAILED)
+    {
+      if (errno == ENOSYS)
+       {
+         puts ("sem_open not supported.  Oh well.");
+         return 0;
+       }
+
+      /* Maybe the shm filesystem has strict permissions.  */
+      if (errno == EACCES)
+       {
+         puts ("sem_open not allowed.  Oh well.");
+         return 0;
+       }
+
+      printf ("sem_open: %m\n");
+      return 1;
+    }
+
+  on_exit (remove_sem, (void *) "/glibc-tst-sem7");
+
+  /* We have the semaphore object.  Now try again.  We should get the
+     same address.  */
+  s2 = sem_open ("/glibc-tst-sem7", O_CREAT, 0600, 1);
+  if (s2 == SEM_FAILED)
+    {
+      puts ("2nd sem_open failed");
+      return 1;
+    }
+  if (s != s2)
+    {
+      puts ("2nd sem_open didn't return the same address");
+      return 1;
+    }
+
+  /* And again, this time without O_CREAT.  */
+  s3 = sem_open ("/glibc-tst-sem7", 0);
+  if (s3 == SEM_FAILED)
+    {
+      puts ("3rd sem_open failed");
+      return 1;
+    }
+  if (s != s3)
+    {
+      puts ("3rd sem_open didn't return the same address");
+      return 1;
+    }
+
+  /* Now close the handle.  Three times.  */
+  if (sem_close (s2) != 0)
+    {
+      puts ("1st sem_close failed");
+      return 1;
+    }
+  if (sem_close (s) != 0)
+    {
+      puts ("2nd sem_close failed");
+      return 1;
+    }
+  if (sem_close (s3) != 0)
+    {
+      puts ("3rd sem_close failed");
+      return 1;
+    }
+
+  return 0;
+}