{"thread":{"id":"25614","subject":"Support pthread with no recursive mutex (SunOS 5.6)","startedAt":"2010-11-02T14:12:27Z","lastAt":"2010-11-04T15:01:04Z","messageCount":3,"participants":["Gary V. Vaughan","Jonathan Nieder","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"154968","messageId":"20101102141227.GA3991@thor.il.thewrittenword.com","threadId":"25614","inReplyTo":null,"subject":"Support pthread with no recursive mutex (SunOS 5.6)","fromName":"Gary V. Vaughan","fromEmail":"git@mlists.thewrittenword.com","sentAt":"2010-11-02T14:12:27Z","receivedAt":"2010-11-02T14:12:27Z","isPatch":false,"sender":{"key":"git@mlists.thewrittenword.com","avatar":null},"body":"Thanks for merging my last patch series into the new release.  git 1.7.3.2\nnow compiles correctly on all of our hosts, save Solaris 2.6 (SunOS 5.6)\nwhich has no recursive mutex support in its pthreads.  While we could\npenalize that platform by disabling threads entirely, they worked perfectly\nwell up until this release.\n\nThis patch adds a new configure and Makefile setting to selectively disable\njust the single recursive mutex usage in favour of the older implementation\nthat used a simple non-recursive mutex in the same spot, but on just Solaris\n2.6.\n\nSigned-off-by: Gary V. Vaughan <gary@thewrittenword.com>\n---\n Makefile               |    4 ++++\n builtin/pack-objects.c |    4 ++++\n config.mak.in          |    1 +\n configure.ac           |   16 ++++++++++++++++\n thread-utils.c         |    2 ++\n thread-utils.h         |    2 ++\n 6 files changed, 29 insertions(+)\n\nIndex: b/Makefile\n===================================================================\n--- a/Makefile\n+++ b/Makefile\n@@ -119,6 +119,9 @@ all::\n #\n # Define NO_PTHREADS if you do not have or do not want to use Pthreads.\n #\n+# Define NO_RECURSIVE_MUTEX if you do have Pthreads, but with no support for\n+# recursive mutexs (SunOS 5.6).\n+#\n # Define NO_PREAD if you have a problem with pread() system call (e.g.\n # cygwin1.dll before v1.5.22).\n #\n@@ -847,6 +850,7 @@ ifeq ($(uname_S),SunOS)\n \t\tSOCKLEN_T = int\n \t\tNO_HSTRERROR = YesPlease\n \t\tNO_IPV6 = YesPlease\n+\t\tNO_RECURSIVE_MUTEX = YesPlease\n \t\tNO_SOCKADDR_STORAGE = YesPlease\n \t\tNO_UNSETENV = YesPlease\n \t\tNO_SETENV = YesPlease\nIndex: b/builtin/pack-objects.c\n===================================================================\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1561,7 +1561,11 @@ static pthread_cond_t progress_cond;\n  */\n static void init_threaded_search(void)\n {\n+#ifndef NO_RECURSIVE_MUTEX\n \tinit_recursive_mutex(&read_mutex);\n+#else\n+\tpthread_mutex_init(&read_mutex, NULL);\n+#endif\n \tpthread_mutex_init(&cache_mutex, NULL);\n \tpthread_mutex_init(&progress_mutex, NULL);\n \tpthread_cond_init(&progress_cond, NULL);\nIndex: b/configure.ac\n===================================================================\n--- a/configure.ac\n+++ b/configure.ac\n@@ -876,6 +876,9 @@ AC_SUBST(NO_MKSTEMPS)\n # Define NO_PTHREADS if we do not have pthreads.\n #\n # Define PTHREAD_LIBS to the linker flag used for Pthread support.\n+#\n+# Define NO_RECURSIVE_MUTEX if you do have Pthreads, but with no support for\n+# recursive mutexes (SunOS 5.6).\n AC_DEFUN([PTHREADTEST_SRC], [\n #include <pthread.h>\n \n@@ -940,9 +943,22 @@ fi\n \n CFLAGS=\"$old_CFLAGS\"\n \n+if test $threads_found = yes; then\n+  AC_MSG_CHECKING([Checking for PTHREAD_MUTEX_RECURSIVE support])\n+  AC_EGREP_HEADER([PTHREAD_MUTEX_RECURSIVE], [pthread.h],\n+\t[], [NO_RECURSIVE_MUTEX=YesPlease])\n+  if test -n \"$NO_RECURSIVE_MUTEX\"; then\n+    AC_MSG_RESULT([no])\n+  else\n+    AC_MSG_RESULT([yes])\n+  fi\n+fi\n+\n+\n AC_SUBST(PTHREAD_CFLAGS)\n AC_SUBST(PTHREAD_LIBS)\n AC_SUBST(NO_PTHREADS)\n+AC_SUBST(NO_RECURSIVE_MUTEX)\n \n ## Output files\n AC_CONFIG_FILES([\"${config_file}\":\"${config_in}\":\"${config_append}\"])\nIndex: b/thread-utils.c\n===================================================================\n--- a/thread-utils.c\n+++ b/thread-utils.c\n@@ -45,6 +45,7 @@ int online_cpus(void)\n \treturn 1;\n }\n \n+#ifndef NO_RECURSIVE_MUTEX\n int init_recursive_mutex(pthread_mutex_t *m)\n {\n \tpthread_mutexattr_t a;\n@@ -59,3 +60,4 @@ int init_recursive_mutex(pthread_mutex_t\n \t}\n \treturn ret;\n }\n+#endif\nIndex: b/thread-utils.h\n===================================================================\n--- a/thread-utils.h\n+++ b/thread-utils.h\n@@ -2,6 +2,8 @@\n #define THREAD_COMPAT_H\n \n extern int online_cpus(void);\n+#ifndef NO_RECURSIVE_MUTEX\n extern int init_recursive_mutex(pthread_mutex_t*);\n+#endif\n \n #endif /* THREAD_COMPAT_H */\nIndex: b/config.mak.in\n===================================================================\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -66,5 +66,6 @@ SOCKLEN_T=@SOCKLEN_T@\n FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@\n SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@\n NO_PTHREADS=@NO_PTHREADS@\n+NO_RECURSIVE_MUTEX=@NO_RECURSIVE_MUTEX@\n PTHREAD_CFLAGS=@PTHREAD_CFLAGS@\n PTHREAD_LIBS=@PTHREAD_LIBS@\n--\nGary V. Vaughan (gary@thewrittenword.com)\n"},{"id":"154989","messageId":"20101102173510.GB5636@burratino","threadId":"25614","inReplyTo":"20101102141227.GA3991@thor.il.thewrittenword.com","subject":"Re: Support pthread with no recursive mutex (SunOS 5.6)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-02T17:35:10Z","receivedAt":"2010-11-02T17:35:10Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Gary,\n\nGary V. Vaughan wrote:\n\n> Thanks for merging my last patch series into the new release.  git 1.7.3.2\n> now compiles correctly on all of our hosts, save Solaris 2.6 (SunOS 5.6)\n> which has no recursive mutex support in its pthreads.\n\nNice.\n\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -1561,7 +1561,11 @@ static pthread_cond_t progress_cond;\n>   */\n>  static void init_threaded_search(void)\n>  {\n> +#ifndef NO_RECURSIVE_MUTEX\n>  \tinit_recursive_mutex(&read_mutex);\n> +#else\n> +\tpthread_mutex_init(&read_mutex, NULL);\n> +#endif\n\nWouldn't that defeat the purpose of using a recursive mutex in the first\nplace?  Let's see...\n\n$ git log -m --first-parent -S'init_recursive_mutex' -- builtin/pack-objects.c\ncommit ea5f75a64ae52590b06713d45d84de03ca109ccc\nMerge: af65543 9374919\nAuthor: Junio C Hamano <gitster@pobox.com>\nDate:   Fri May 21 04:02:16 2010 -0700\n\n    Merge branch 'np/malloc-threading'\n    \n    * np/malloc-threading:\n      Thread-safe xmalloc and xrealloc needs a recursive mutex\n      Make xmalloc and xrealloc thread-safe\n$ git log -S'init_recursive_mutex' af65543..9374919\ncommit 937491944292fa3303b565b9bd8914c6b644ab13\nAuthor: Johannes Sixt <j6t@kdbg.org>\nDate:   Thu Apr 8 09:15:39 2010 +0200\n\n    Thread-safe xmalloc and xrealloc needs a recursive mutex\n    \n    The mutex used to protect object access (read_mutex) may need to be\n    acquired recursively.  Introduce init_recursive_mutex() helper function\n    in thread-utils.c that constructs a mutex with the PHREAD_MUTEX_RECURSIVE\n    attribute.\n    \n    pthread_mutex_init() emulation on Win32 is already recursive as it is\n    implemented on top of the CRITICAL_SECTION type, which is recursive.\n    \n        http://msdn.microsoft.com/en-us/library/ms682530%28VS.85%29.aspx\n    \n    Add do-nothing compatibility wrappers for pthread_mutexattr* functions.\n    \n    Initial-version-by: Fredrik Kuivinen <frekui@gmail.com>\n    Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nPresumably the problem scenarios are something like this:\n\n\tll_find_deltas():\n\t  init_threaded_search() [sets try_to_free_routine]\n\t  threaded_find_deltas() ->\n\t   find_deltas() ->\n\t    try_delta() [acquires read_lock] ->\n\t     read_sha1_file() ->\n\t      read_object() ->\n\t       xmemdupz() ->\n\t        xmallocz() ->\n\t         xmalloc() ->\n\t          try_to_free_from_threads() ->\n\t           read_lock() --- deadlock.\n\nHope that helps,\nJonathan\n"},{"id":"155173","messageId":"7vvd4duo9b.fsf@alter.siamese.dyndns.org","threadId":"25614","inReplyTo":"20101102173510.GB5636@burratino","subject":"Re: Support pthread with no recursive mutex (SunOS 5.6)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-04T15:01:04Z","receivedAt":"2010-11-04T15:01:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Hi Gary,\n>\n> Gary V. Vaughan wrote:\n>\n>> Thanks for merging my last patch series into the new release.  git 1.7.3.2\n>> now compiles correctly on all of our hosts, save Solaris 2.6 (SunOS 5.6)\n>> which has no recursive mutex support in its pthreads.\n>\n> Nice.\n>\n>> --- a/builtin/pack-objects.c\n>> +++ b/builtin/pack-objects.c\n>> @@ -1561,7 +1561,11 @@ static pthread_cond_t progress_cond;\n>>   */\n>>  static void init_threaded_search(void)\n>>  {\n>> +#ifndef NO_RECURSIVE_MUTEX\n>>  \tinit_recursive_mutex(&read_mutex);\n>> +#else\n>> +\tpthread_mutex_init(&read_mutex, NULL);\n>> +#endif\n>\n> Wouldn't that defeat the purpose of using a recursive mutex in the first\n> place?\n\nThanks for a sanity.\n\nWhat might make sense is not NO_RECURSIVE_MUTEX but MUTEX_IS_RECURSIVE\n(which would be the Windows case).  If Solaris 2.6 has mutex without\nPTHREAD_MUTEX_RECURSIVE, and its mutex is already recursive without being\ntold anything special, then something like the above patch (#ifdef should\nbe in the definition of init_recursive_mutex() function, not its callsite,\nby the way) would be a good thing to have, but I somehow doubt that it\nwould be the case...\n"}]}