From 6b9dcf87ecc79d795e79e3d6d8e03b894f36a6cc Mon Sep 17 00:00:00 2001 From: oehhar Date: Wed, 17 Jul 2013 16:02:36 +0000 Subject: Start notifier thread again if we were forked, to solve Rivet bug 55153 - RFE [a0bc856dcd] --- ChangeLog | 6 ++++++ unix/tclUnixNotfy.c | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/ChangeLog b/ChangeLog index d7ada95..bdbbd4a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2013-07-17 Harald Oehlmann + + * tclUnixNotify.c Tcl_InitNotifier: RFE [a0bc856dcd] + Start notifier thread again if we were forked, to solve Rivet bug + 55153. + 2013-07-05 Kevin B. Kenny * library/tzdata/Africa/Casablanca: diff --git a/unix/tclUnixNotfy.c b/unix/tclUnixNotfy.c index aacc8d2d..2ad1932 100644 --- a/unix/tclUnixNotfy.c +++ b/unix/tclUnixNotfy.c @@ -120,6 +120,15 @@ static Tcl_ThreadDataKey dataKey; static int notifierCount = 0; /* + * The following static stores the process ID of the initialized notifier + * thread. If it changes, we have passed a fork and we should start a new + * notifier thread. + * + * You must hold the notifierMutex lock before accessing this variable. + */ +static pid_t processIDInitialized = 0; + +/* * The following variable points to the head of a doubly-linked list of * ThreadSpecificData structures for all threads that are currently waiting on * an event. @@ -275,11 +284,23 @@ Tcl_InitNotifier(void) */ Tcl_MutexLock(¬ifierMutex); + /* + * Check if my process id changed, e.g. I was forked + * In this case, restart the notifier thread and close the + * pipe to the original notifier thread + */ + if (notifierCount > 0 && processIDInitialized != getpid()) { + notifierCount = 0; + processIDInitialized = 0; + close(triggerPipe); + triggerPipe = -1; + } if (notifierCount == 0) { if (TclpThreadCreate(¬ifierThread, NotifierThreadProc, NULL, TCL_THREAD_STACK_DEFAULT, TCL_THREAD_JOINABLE) != TCL_OK) { Tcl_Panic("Tcl_InitNotifier: unable to start notifier thread"); } + processIDInitialized = getpid(); } notifierCount++; -- cgit v0.12 From ec06295d4ed7d6f865b8d73925bfb24d8f478f45 Mon Sep 17 00:00:00 2001 From: oehhar Date: Mon, 22 Jul 2013 08:45:37 +0000 Subject: Test file tests/unixForkEvent.test added --- tests/unixForkEvent.test | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 tests/unixForkEvent.test diff --git a/tests/unixForkEvent.test b/tests/unixForkEvent.test new file mode 100644 index 0000000..fee45fd --- /dev/null +++ b/tests/unixForkEvent.test @@ -0,0 +1,50 @@ +# This file contains a collection of tests for the procedures in the file +# tclEvent.c, which includes the "update", and "vwait" Tcl +# commands. Sourcing this file into Tcl runs the tests and generates +# output for errors. No output means no errors were found. +# +# Copyright (c) 1995-1997 Sun Microsystems, Inc. +# Copyright (c) 1998-1999 by Scriptics Corporation. +# +# See the file "license.terms" for information on usage and redistribution +# of this file, and for a DISCLAIMER OF ALL WARRANTIES. + +package require tcltest 2 +namespace import -force ::tcltest::* + +testConstraint testfork [llength [info commands testfork]] +testConstraint threaded [expr { + ([info exist tcl_platform(threaded)] && $tcl_platform(threaded)) + && $tcl_platform(os) ne "Darwin" +}] + +# Test if the notifier thread is well initialized in a forked interpreter +# by Tcl_InitNotifier +test unixforkevent-1.1 {fork and test writeable event} \ + -constraints {threaded testfork} \ + -body { + set folder [makeDirectory unixtestfork] + set pid [testfork] + if {$pid == 0} { + # we are the forked process + set result initialized + set h [open [file join $folder test.txt] w] + fileevent $h writable\ + "set result writable;\ + after cancel [after 1000 {set result timeout}]" + vwait result + close $h + makeFile $result result.txt $folder + exit + } + # we are the original process + while {![file readable [file join $folder result.txt]]} {} + viewFile result.txt $folder + } \ + -result {writable} \ + -cleanup { + catch { removeFolder $folder } + } + +::tcltest::cleanupTests +return -- cgit v0.12 From 978b54229dc68812a5ccaa0050324d215db91520 Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Mon, 22 Jul 2013 10:10:00 +0000 Subject: Fix bug which hangs iocmd.tf-32.1 --- unix/tclUnixNotfy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unix/tclUnixNotfy.c b/unix/tclUnixNotfy.c index 88e290e..6e8b5fa 100644 --- a/unix/tclUnixNotfy.c +++ b/unix/tclUnixNotfy.c @@ -291,8 +291,8 @@ Tcl_InitNotifier(void) if (TclpThreadCreate(¬ifierThread, NotifierThreadProc, NULL, TCL_THREAD_STACK_DEFAULT, TCL_THREAD_JOINABLE) != TCL_OK) { Tcl_Panic("Tcl_InitNotifier: unable to start notifier thread"); - processIDInitialized = getpid(); } + processIDInitialized = getpid(); } notifierCount++; -- cgit v0.12 From f9820a2863b758aa1d07274355cd62f9ecbddae1 Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Mon, 22 Jul 2013 10:11:05 +0000 Subject: Test-case should pass on Darwin or with non-threaded build as well. --- tests/unixForkEvent.test | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/unixForkEvent.test b/tests/unixForkEvent.test index fee45fd..a0c3f19 100644 --- a/tests/unixForkEvent.test +++ b/tests/unixForkEvent.test @@ -1,7 +1,6 @@ # This file contains a collection of tests for the procedures in the file -# tclEvent.c, which includes the "update", and "vwait" Tcl -# commands. Sourcing this file into Tcl runs the tests and generates -# output for errors. No output means no errors were found. +# tclUnixNotify.c. Sourcing this file into Tcl runs the tests and +# generates output for errors. No output means no errors were found. # # Copyright (c) 1995-1997 Sun Microsystems, Inc. # Copyright (c) 1998-1999 by Scriptics Corporation. @@ -13,15 +12,11 @@ package require tcltest 2 namespace import -force ::tcltest::* testConstraint testfork [llength [info commands testfork]] -testConstraint threaded [expr { - ([info exist tcl_platform(threaded)] && $tcl_platform(threaded)) - && $tcl_platform(os) ne "Darwin" -}] # Test if the notifier thread is well initialized in a forked interpreter # by Tcl_InitNotifier test unixforkevent-1.1 {fork and test writeable event} \ - -constraints {threaded testfork} \ + -constraints testfork \ -body { set folder [makeDirectory unixtestfork] set pid [testfork] -- cgit v0.12 From a038288139470e7a32c1e711fcfdbd64212e87e4 Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Mon, 22 Jul 2013 10:57:44 +0000 Subject: Use pthread_atfork() when available. --- unix/configure | 209 +++++++++++++++++++++++++++------------------------- unix/configure.in | 7 +- unix/tclUnixNotfy.c | 90 +++++++++++++++++++++- 3 files changed, 202 insertions(+), 104 deletions(-) diff --git a/unix/configure b/unix/configure index 6faa7d8..ee194b7 100755 --- a/unix/configure +++ b/unix/configure @@ -16759,6 +16759,113 @@ done #-------------------------------------------------------------------- +# Check for support of pthread_atfork function +#-------------------------------------------------------------------- + + +for ac_func in pthread_atfork +do +as_ac_var=`echo "ac_cv_func_$ac_func" | $as_tr_sh` +echo "$as_me:$LINENO: checking for $ac_func" >&5 +echo $ECHO_N "checking for $ac_func... $ECHO_C" >&6 +if eval "test \"\${$as_ac_var+set}\" = set"; then + echo $ECHO_N "(cached) $ECHO_C" >&6 +else + cat >conftest.$ac_ext <<_ACEOF +/* confdefs.h. */ +_ACEOF +cat confdefs.h >>conftest.$ac_ext +cat >>conftest.$ac_ext <<_ACEOF +/* end confdefs.h. */ +/* Define $ac_func to an innocuous variant, in case declares $ac_func. + For example, HP-UX 11i declares gettimeofday. */ +#define $ac_func innocuous_$ac_func + +/* System header to define __stub macros and hopefully few prototypes, + which can conflict with char $ac_func (); below. + Prefer to if __STDC__ is defined, since + exists even on freestanding compilers. */ + +#ifdef __STDC__ +# include +#else +# include +#endif + +#undef $ac_func + +/* Override any gcc2 internal prototype to avoid an error. */ +#ifdef __cplusplus +extern "C" +{ +#endif +/* We use char because int might match the return type of a gcc2 + builtin and then its argument prototype would still apply. */ +char $ac_func (); +/* The GNU C library defines this for functions which it implements + to always fail with ENOSYS. Some functions are actually named + something starting with __ and the normal name is an alias. */ +#if defined (__stub_$ac_func) || defined (__stub___$ac_func) +choke me +#else +char (*f) () = $ac_func; +#endif +#ifdef __cplusplus +} +#endif + +int +main () +{ +return f != $ac_func; + ; + return 0; +} +_ACEOF +rm -f conftest.$ac_objext conftest$ac_exeext +if { (eval echo "$as_me:$LINENO: \"$ac_link\"") >&5 + (eval $ac_link) 2>conftest.er1 + ac_status=$? + grep -v '^ *+' conftest.er1 >conftest.err + rm -f conftest.er1 + cat conftest.err >&5 + echo "$as_me:$LINENO: \$? = $ac_status" >&5 + (exit $ac_status); } && + { ac_try='test -z "$ac_c_werror_flag" + || test ! -s conftest.err' + { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5 + (eval $ac_try) 2>&5 + ac_status=$? + echo "$as_me:$LINENO: \$? = $ac_status" >&5 + (exit $ac_status); }; } && + { ac_try='test -s conftest$ac_exeext' + { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5 + (eval $ac_try) 2>&5 + ac_status=$? + echo "$as_me:$LINENO: \$? = $ac_status" >&5 + (exit $ac_status); }; }; then + eval "$as_ac_var=yes" +else + echo "$as_me: failed program was:" >&5 +sed 's/^/| /' conftest.$ac_ext >&5 + +eval "$as_ac_var=no" +fi +rm -f conftest.err conftest.$ac_objext \ + conftest$ac_exeext conftest.$ac_ext +fi +echo "$as_me:$LINENO: result: `eval echo '${'$as_ac_var'}'`" >&5 +echo "${ECHO_T}`eval echo '${'$as_ac_var'}'`" >&6 +if test `eval echo '${'$as_ac_var'}'` = yes; then + cat >>confdefs.h <<_ACEOF +#define `echo "HAVE_$ac_func" | $as_tr_cpp` 1 +_ACEOF + +fi +done + + +#-------------------------------------------------------------------- # Check for support of isnan() function or macro #-------------------------------------------------------------------- @@ -17439,108 +17546,6 @@ _ACEOF fi done - -for ac_func in pthread_atfork -do -as_ac_var=`echo "ac_cv_func_$ac_func" | $as_tr_sh` -echo "$as_me:$LINENO: checking for $ac_func" >&5 -echo $ECHO_N "checking for $ac_func... $ECHO_C" >&6 -if eval "test \"\${$as_ac_var+set}\" = set"; then - echo $ECHO_N "(cached) $ECHO_C" >&6 -else - cat >conftest.$ac_ext <<_ACEOF -/* confdefs.h. */ -_ACEOF -cat confdefs.h >>conftest.$ac_ext -cat >>conftest.$ac_ext <<_ACEOF -/* end confdefs.h. */ -/* Define $ac_func to an innocuous variant, in case declares $ac_func. - For example, HP-UX 11i declares gettimeofday. */ -#define $ac_func innocuous_$ac_func - -/* System header to define __stub macros and hopefully few prototypes, - which can conflict with char $ac_func (); below. - Prefer to if __STDC__ is defined, since - exists even on freestanding compilers. */ - -#ifdef __STDC__ -# include -#else -# include -#endif - -#undef $ac_func - -/* Override any gcc2 internal prototype to avoid an error. */ -#ifdef __cplusplus -extern "C" -{ -#endif -/* We use char because int might match the return type of a gcc2 - builtin and then its argument prototype would still apply. */ -char $ac_func (); -/* The GNU C library defines this for functions which it implements - to always fail with ENOSYS. Some functions are actually named - something starting with __ and the normal name is an alias. */ -#if defined (__stub_$ac_func) || defined (__stub___$ac_func) -choke me -#else -char (*f) () = $ac_func; -#endif -#ifdef __cplusplus -} -#endif - -int -main () -{ -return f != $ac_func; - ; - return 0; -} -_ACEOF -rm -f conftest.$ac_objext conftest$ac_exeext -if { (eval echo "$as_me:$LINENO: \"$ac_link\"") >&5 - (eval $ac_link) 2>conftest.er1 - ac_status=$? - grep -v '^ *+' conftest.er1 >conftest.err - rm -f conftest.er1 - cat conftest.err >&5 - echo "$as_me:$LINENO: \$? = $ac_status" >&5 - (exit $ac_status); } && - { ac_try='test -z "$ac_c_werror_flag" - || test ! -s conftest.err' - { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5 - (eval $ac_try) 2>&5 - ac_status=$? - echo "$as_me:$LINENO: \$? = $ac_status" >&5 - (exit $ac_status); }; } && - { ac_try='test -s conftest$ac_exeext' - { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5 - (eval $ac_try) 2>&5 - ac_status=$? - echo "$as_me:$LINENO: \$? = $ac_status" >&5 - (exit $ac_status); }; }; then - eval "$as_ac_var=yes" -else - echo "$as_me: failed program was:" >&5 -sed 's/^/| /' conftest.$ac_ext >&5 - -eval "$as_ac_var=no" -fi -rm -f conftest.err conftest.$ac_objext \ - conftest$ac_exeext conftest.$ac_ext -fi -echo "$as_me:$LINENO: result: `eval echo '${'$as_ac_var'}'`" >&5 -echo "${ECHO_T}`eval echo '${'$as_ac_var'}'`" >&6 -if test `eval echo '${'$as_ac_var'}'` = yes; then - cat >>confdefs.h <<_ACEOF -#define `echo "HAVE_$ac_func" | $as_tr_cpp` 1 -_ACEOF - -fi -done - fi cat >>confdefs.h <<\_ACEOF diff --git a/unix/configure.in b/unix/configure.in index e22a7d3..f47b882 100644 --- a/unix/configure.in +++ b/unix/configure.in @@ -491,6 +491,12 @@ SC_ENABLE_LANGINFO AC_CHECK_FUNCS(chflags) #-------------------------------------------------------------------- +# Check for support of pthread_atfork function +#-------------------------------------------------------------------- + +AC_CHECK_FUNCS(pthread_atfork) + +#-------------------------------------------------------------------- # Check for support of isnan() function or macro #-------------------------------------------------------------------- @@ -513,7 +519,6 @@ if test "`uname -s`" = "Darwin" ; then if test $tcl_corefoundation = yes; then AC_CHECK_HEADERS(libkern/OSAtomic.h) AC_CHECK_FUNCS(OSSpinLockLock) - AC_CHECK_FUNCS(pthread_atfork) fi AC_DEFINE(USE_VFORK, 1, [Should we use vfork() instead of fork()?]) AC_DEFINE(TCL_DEFAULT_ENCODING, "utf-8", diff --git a/unix/tclUnixNotfy.c b/unix/tclUnixNotfy.c index 6e8b5fa..f414c3f 100644 --- a/unix/tclUnixNotfy.c +++ b/unix/tclUnixNotfy.c @@ -202,7 +202,13 @@ static Tcl_ThreadId notifierThread; #ifdef TCL_THREADS static void NotifierThreadProc(ClientData clientData); -#endif +#ifdef HAVE_PTHREAD_ATFORK +static int atForkInit = 0; +static void AtForkPrepare(void); +static void AtForkParent(void); +static void AtForkChild(void); +#endif /* HAVE_PTHREAD_ATFORK */ +#endif /* TCL_THREADS */ static int FileHandlerEventProc(Tcl_Event *evPtr, int flags); /* @@ -276,6 +282,21 @@ Tcl_InitNotifier(void) */ Tcl_MutexLock(¬ifierMutex); +#ifdef HAVE_PTHREAD_ATFORK + /* + * Install pthread_atfork handlers to reinitialize the notifier in the + * child of a fork. + */ + + if (!atForkInit) { + int result = pthread_atfork(AtForkPrepare, AtForkParent, AtForkChild); + + if (result) { + Tcl_Panic("Tcl_InitNotifier: pthread_atfork failed"); + } + atForkInit = 1; + } +#endif /* * Check if my process id changed, e.g. I was forked * In this case, restart the notifier thread and close the @@ -1251,6 +1272,73 @@ NotifierThreadProc( TclpThreadExit (0); } + +#ifdef HAVE_PTHREAD_ATFORK +/* + *---------------------------------------------------------------------- + * + * AtForkPrepare -- + * + * Lock the notifier in preparation for a fork. + * + * Results: + * None. + * + * Side effects: + * None. + * + *---------------------------------------------------------------------- + */ + +static void +AtForkPrepare(void) +{ +} + +/* + *---------------------------------------------------------------------- + * + * AtForkParent -- + * + * Unlock the notifier in the parent after a fork. + * + * Results: + * None. + * + * Side effects: + * None. + * + *---------------------------------------------------------------------- + */ + +static void +AtForkParent(void) +{ +} + +/* + *---------------------------------------------------------------------- + * + * AtForkChild -- + * + * Unlock and reinstall the notifier in the child after a fork. + * + * Results: + * None. + * + * Side effects: + * None. + * + *---------------------------------------------------------------------- + */ + +static void +AtForkChild(void) +{ + Tcl_InitNotifier(); +} +#endif /* HAVE_PTHREAD_ATFORK */ + #endif /* TCL_THREADS */ #endif /* HAVE_COREFOUNDATION */ -- cgit v0.12 From 7a32d8f6e8b39e52c94b3a375a95bd3b6669c12e Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Mon, 22 Jul 2013 11:11:10 +0000 Subject: Tcl_InitNotifier() call in TestforkObjCmd() is only necessary when pthread_atfork() is not available. --- unix/tclUnixTest.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/unix/tclUnixTest.c b/unix/tclUnixTest.c index 39cddef..2fc1647 100644 --- a/unix/tclUnixTest.c +++ b/unix/tclUnixTest.c @@ -574,9 +574,12 @@ TestforkObjCmd( "Cannot fork", NULL); return TCL_ERROR; } +#ifndef HAVE_PTHREAD_ATFORK + /* Only needed when pthread_atfork is not present. */ if (pid==0) { Tcl_InitNotifier(); } +#endif Tcl_SetObjResult(interp, Tcl_NewIntObj(pid)); return TCL_OK; } -- cgit v0.12 From 69fbd6c489ee246d40067e6a6394419ecb9a6b07 Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Thu, 25 Jul 2013 08:29:56 +0000 Subject: Make sure that the notifierMutex and notifierCV in a forked child cannot block anything, even though the initialization of the Notifier Thread in the parent is not finished yet. --- unix/tclUnixNotfy.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/unix/tclUnixNotfy.c b/unix/tclUnixNotfy.c index f414c3f..ec721ca 100644 --- a/unix/tclUnixNotfy.c +++ b/unix/tclUnixNotfy.c @@ -1335,6 +1335,8 @@ AtForkParent(void) static void AtForkChild(void) { + notifierMutex = NULL; + notifierCV = NULL; Tcl_InitNotifier(); } #endif /* HAVE_PTHREAD_ATFORK */ -- cgit v0.12 From 9b087e12907c940eed87b510213ca363ef8d5a4b Mon Sep 17 00:00:00 2001 From: oehhar Date: Thu, 25 Jul 2013 17:20:02 +0000 Subject: Fixed test case variable clash with 'folder' --- tests/unixForkEvent.test | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unixForkEvent.test b/tests/unixForkEvent.test index a0c3f19..cbe582e 100644 --- a/tests/unixForkEvent.test +++ b/tests/unixForkEvent.test @@ -18,27 +18,27 @@ testConstraint testfork [llength [info commands testfork]] test unixforkevent-1.1 {fork and test writeable event} \ -constraints testfork \ -body { - set folder [makeDirectory unixtestfork] + set myFolder [makeDirectory unixtestfork] set pid [testfork] if {$pid == 0} { # we are the forked process set result initialized - set h [open [file join $folder test.txt] w] + set h [open [file join $myFolder test.txt] w] fileevent $h writable\ "set result writable;\ after cancel [after 1000 {set result timeout}]" vwait result close $h - makeFile $result result.txt $folder + makeFile $result result.txt $myFolder exit } # we are the original process - while {![file readable [file join $folder result.txt]]} {} - viewFile result.txt $folder + while {![file readable [file join $myFolder result.txt]]} {} + viewFile result.txt $myFolder } \ -result {writable} \ -cleanup { - catch { removeFolder $folder } + catch { removeFolder $myFolder } } ::tcltest::cleanupTests -- cgit v0.12