Browse Source

Fixes rare race condition in uuid generation.

Brian Aker 12 years ago
parent
commit
dc0f85d7f1
10 changed files with 205 additions and 10 deletions
  1. 2 1
      ChangeLog
  2. 3 3
      configure.ac
  3. 2 0
      docs/gearmand.rst
  4. 4 4
      libgearman/add.cc
  5. 3 1
      libgearman/include.am
  6. 51 0
      libgearman/uuid.cc
  7. 42 0
      libgearman/uuid.hpp
  8. 1 1
      m4/ac_cxx_compile_stdcxx_0x.m4
  9. 49 0
      m4/ax_pthread_timedjoin_np.m4
  10. 48 0
      m4/uuid.m4

+ 2 - 1
ChangeLog

@@ -1,6 +1,7 @@
-0.36
+0.36 
 * Fix issue where sqlite was not correctly being shutdown.
 * Check for more warnings (and cleanup found from).
+* Fix thread safety issue for uuid_generate().
 
 0.35 Sun Aug  5 16:03:21 EDT 2012
 * Critical bug fix for rare case in which initialization of pipe for a worker fails.

+ 3 - 3
configure.ac

@@ -165,9 +165,6 @@ AC_CHECK_FUNCS([dup2 gettimeofday memchr memmove memset socket strcasecmp strdup
 AC_CHECK_FUNCS([pipe2], 
                AC_DEFINE([HAVE_PIPE2], [1], [Have pipe2]),
                AC_DEFINE([HAVE_PIPE2], [0], [Have pipe2]))
-AC_CHECK_FUNCS([pthread_timedjoin_np], 
-               AC_DEFINE([HAVE_PTHREAD_TIMEDJOIN_NP], [1], [Have pthread_timedjoin_np]),
-               AC_DEFINE([HAVE_PTHREAD_TIMEDJOIN_NP], [0], [Have pthread_timedjoin_np]))
 AC_CHECK_FUNCS([select])
 AC_CHECK_FUNCS([setenv])
 AC_CHECK_FUNCS([strtol])
@@ -224,6 +221,7 @@ AX_CHECK_LIBRARY([LIBEVENT], [event.h], [event],
 AX_CHECK_LIBRARY([LIBUUID], [uuid/uuid.h], [uuid], 
                  [
                   LIBUUID_LDFLAGS="-luuid"
+                  AX_UUID_GENERATE_TIME_SAFE
                   ],
                  [
                   AC_MSG_WARN([Unable to find libuuid])
@@ -236,6 +234,8 @@ AX_CHECK_SOCK_CLOEXEC([AC_DEFINE([HAVE_SOCK_CLOEXEC], [1], [Check for SOCK_CLOEX
                        [AC_DEFINE([HAVE_SOCK_CLOEXEC], [0], [Check for SOCK_CLOEXEC.])])
 
 AX_PTHREAD(, [AC_MSG_ERROR(could not find libpthread)])
+AX_PTHREAD_TIMEDJOIN_NP
+
 AX_HARDEN_COMPILER_FLAGS
 
 AX_DEBUG

+ 2 - 0
docs/gearmand.rst

@@ -197,6 +197,8 @@ The I/O thread is responsible for doing the read and write system calls on the s
 
 The processing thread should have no system calls within it (except for the occasional brk() for more memory), and manages the various lists and hash tables used for tracking unique keys, job handles, functions, and job queues. All packets that need to be sent back to connections are put into an asynchronous queue for the I/O thread. The I/O thread will pick these up and send them back over the connected socket. All packets flow through the processing thread since it contains the information needed to process the packets. This is due to the complex nature of the various lists and hash tables. If multiple threads were modifying them the locking overhead would most likely cause worse performance than having it in a single thread (and would also complicate the code). In the future more work may be pushed to the I/O threads, and the processing thread can retain minimal functionality to manage those tables and lists. So far this has not been a significant bottleneck, a 16 core Intel machine is able to process upwards of 50k jobs per second.
 
+For thread safety to work when UUID are generated, you must be running the uuidd daemon.
+
 Persistent Queues
 +++++++++++++++++
 

+ 4 - 4
libgearman/add.cc

@@ -45,14 +45,14 @@
 
 #include "libgearman/assert.hpp"
 
+#include "libgearman/uuid.hpp"
+
 #include <cerrno>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <memory>
 
-#include <uuid/uuid.h>
-
 gearman_task_st *add_task(gearman_client_st& client,
                           void *context,
                           gearman_command_t command,
@@ -153,7 +153,7 @@ gearman_task_st *add_task(gearman_client_st& client,
   else
   {
     uuid_t uuid;
-    uuid_generate(uuid);
+    safe_uuid_generate(uuid);
     uuid_unparse(uuid, task->unique);
     task->unique_length= GEARMAN_MAX_UUID_SIZE;
   }
@@ -332,7 +332,7 @@ gearman_task_st *add_reducer_task(gearman_client_st *client,
   }
   else
   {
-    uuid_generate(uuid);
+    safe_uuid_generate(uuid);
     uuid_unparse(uuid, task->unique);
     task->unique[GEARMAN_MAX_UUID_SIZE]= 0;
     args[1]= task->unique;

+ 3 - 1
libgearman/include.am

@@ -1,6 +1,6 @@
 # vim:ft=automake
 # Gearman server and library 
-# Copyright (C) 2011 Data Differential, http://datadifferential.com/
+# Copyright (C) 2011 - 2012 Data Differential, http://datadifferential.com/
 # Copyright (C) 2009-2010 Brian Aker, Eric Day, Monty Taylor All rights
 # Copyright (C) 2008 Brian Aker, Eric Day 
 #
@@ -12,6 +12,7 @@
 
 nobase_include_HEADERS+= libgearman/gearman.h
 
+noinst_HEADERS+= libgearman/uuid.hpp
 noinst_HEADERS+= libgearman/job.h
 noinst_HEADERS+= libgearman/pipe.h
 noinst_HEADERS+= \
@@ -104,6 +105,7 @@ libgearman_libgearman_la_SOURCES= \
 				  libgearman/worker.cc
 
 libgearman_libgearman_la_SOURCES+= libgearman/pipe.cc
+libgearman_libgearman_la_SOURCES+= libgearman/uuid.cc
 
 libgearman_libgearman_la_CXXFLAGS= -DBUILDING_LIBGEARMAN
 

+ 51 - 0
libgearman/uuid.cc

@@ -0,0 +1,51 @@
+/*  vim:expandtab:shiftwidth=2:tabstop=2:smarttab:
+ * 
+ *  Gearmand client and server library.
+ *
+ *  Copyright (C) 2012 Data Differential, http://datadifferential.com/
+ *  All rights reserved.
+ *
+ *  Redistribution and use in source and binary forms, with or without
+ *  modification, are permitted provided that the following conditions are
+ *  met:
+ *
+ *      * Redistributions of source code must retain the above copyright
+ *  notice, this list of conditions and the following disclaimer.
+ *
+ *      * Redistributions in binary form must reproduce the above
+ *  copyright notice, this list of conditions and the following disclaimer
+ *  in the documentation and/or other materials provided with the
+ *  distribution.
+ *
+ *      * The names of its contributors may not be used to endorse or
+ *  promote products derived from this software without specific prior
+ *  written permission.
+ *
+ *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *  A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *  OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *  SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *  LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *  DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *  THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#include "config.h"
+
+#include "libgearman/uuid.hpp"
+
+int safe_uuid_generate(uuid_t out)
+{
+#if defined(HAVE_UUID_GENERATE_TIME_SAFE) && HAVE_UUID_GENERATE_TIME_SAFE
+  return uuid_generate_time_safe(out);
+#else
+  uuid_generate(out);
+  return -1;
+#endif
+}
+

+ 42 - 0
libgearman/uuid.hpp

@@ -0,0 +1,42 @@
+/*  vim:expandtab:shiftwidth=2:tabstop=2:smarttab:
+ * 
+ *  Gearmand client and server library.
+ *
+ *  Copyright (C) 2012 Data Differential, http://datadifferential.com/
+ *  All rights reserved.
+ *
+ *  Redistribution and use in source and binary forms, with or without
+ *  modification, are permitted provided that the following conditions are
+ *  met:
+ *
+ *      * Redistributions of source code must retain the above copyright
+ *  notice, this list of conditions and the following disclaimer.
+ *
+ *      * Redistributions in binary form must reproduce the above
+ *  copyright notice, this list of conditions and the following disclaimer
+ *  in the documentation and/or other materials provided with the
+ *  distribution.
+ *
+ *      * The names of its contributors may not be used to endorse or
+ *  promote products derived from this software without specific prior
+ *  written permission.
+ *
+ *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *  A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *  OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *  SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *  LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *  DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *  THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#pragma once
+
+#include <uuid/uuid.h>
+
+int safe_uuid_generate(uuid_t out);

+ 1 - 1
m4/ac_cxx_compile_stdcxx_0x.m4

@@ -23,7 +23,7 @@ AC_DEFUN([AC_CXX_COMPILE_STDCXX_0X], [
   AC_CACHE_CHECK(if g++ supports C++0x features without additional flags,
   ac_cv_cxx_compile_cxx0x_native,
   [AC_LANG_SAVE
-  AC_LANG_CPLUSPLUS
+  AC_LANG_C
   AC_TRY_COMPILE([
   template <typename T>
     struct check

+ 49 - 0
m4/ax_pthread_timedjoin_np.m4

@@ -0,0 +1,49 @@
+# ===========================================================================
+# http://
+# ===========================================================================
+#
+# SYNOPSIS
+#
+#   AX_LIB_UUID
+#
+# DESCRIPTION
+#
+#   Check for pthread_timedjoin_np support.
+#
+# LICENSE
+#
+#   Copyright (c) 2012 Brian Aker <brian@tangent.org>
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved. This file is offered as-is, without any
+#   warranty.
+
+#serial 1
+
+AC_DEFUN([AX_PTHREAD_TIMEDJOIN_NP], [
+    AC_CACHE_CHECK([check for pthread_timedjoin_np], [ax_cv_pthread_timedjoin_np], [
+      save_LDFLAGS= $LDFLAGS
+      LDFLAGS=$(PTHREAD_LIBS)
+      AC_LANG_PUSH([C])
+      AC_COMPILE_IFELSE([
+        AC_LANG_PROGRAM([
+#include <uuid/uuid.h>
+      ], [
+      uuid_t out;
+      pthread_timedjoin_np(out);
+      ]), [ax_cv_pthread_timedjoin_np=yes ], [ax_cv_pthread_timedjoin_np=no ]
+      ])
+
+    AC_LANG_POP
+
+    ])
+
+    AS_IF([test "$ax_cv_pthread_timedjoin_np" = yes],[
+      AC_DEFINE(HAVE_PTHREAD_TIMEDJOIN_NP,[1],[Define if pthread_timedjoin_np is present in pthread.h.])],[
+      AC_DEFINE(HAVE_PTHREAD_TIMEDJOIN_NP,[0],[Define if pthread_timedjoin_np is present in pthread.h.])
+    ])
+
+  LDFLAGS= $save_LDFLAGS
+])
+

+ 48 - 0
m4/uuid.m4

@@ -0,0 +1,48 @@
+# ===========================================================================
+# http://
+# ===========================================================================
+#
+# SYNOPSIS
+#
+#   AX_LIB_UUID
+#
+# DESCRIPTION
+#
+#   Check for uuid, and uuid_generate_time_safe support.
+#
+# LICENSE
+#
+#   Copyright (c) 2012 Brian Aker <brian@tangent.org>
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved. This file is offered as-is, without any
+#   warranty.
+
+#serial 1
+
+AC_DEFUN([AX_UUID_GENERATE_TIME_SAFE], [
+    AC_CACHE_CHECK([check for uuid_generate_time_safe], [ax_cv_uuid_generate_time_safe], [
+      save_LDFLAGS= $LDFLAGS
+      LDFLAGS="-luuid"
+      AC_LANG_PUSH([C])
+      AC_COMPILE_IFELSE([
+        AC_LANG_PROGRAM([
+#include <uuid/uuid.h>
+      ], [
+      uuid_t out;
+      uuid_generate_time_safe(out);
+      ]), [ax_cv_uuid_generate_time_safe=yes ], [ax_cv_uuid_generate_time_safe=no ]
+      ])
+
+    AC_LANG_POP
+
+    ])
+
+  AS_IF([test "$ax_cv_uuid_generate_time_safe" = yes],[
+    AC_DEFINE(HAVE_UUID_GENERATE_TIME_SAFE,[1],[Define if uuid_generate_time_safe is present in uuid/uuid.h.])],[
+    AC_DEFINE(HAVE_UUID_GENERATE_TIME_SAFE,[0],[Define if uuid_generate_time_safe is present in uuid/uuid.h.])
+  ])
+
+  LDFLAGS= $save_LDFLAGS
+])