runtime/cgo: avoid races on cgo_context_function

Change-Id: Ie9e6fda675e560234e90b9022526fd689d770818
Reviewed-on: https://go-review.googlesource.com/23610
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/runtime/cgo/gcc_context.c b/src/runtime/cgo/gcc_context.c
index 81556cd464724c484e928bfe6cb36edac3f19342..1e6cf7ee1286fdc19450a0dac1f8b9c012f330bf 100644 (file)
--- a/src/runtime/cgo/gcc_context.c
+++ b/src/runtime/cgo/gcc_context.c
@@ -7,21 +7,15 @@
 
 #include "libcgo.h"
 
-// The context function, used when tracing back C calls into Go.
-void (*x_cgo_context_function)(struct context_arg*);
-
-// Sets the context function to call to record the traceback context
-// when calling a Go function from C code. Called from runtime.SetCgoTraceback.
-void x_cgo_set_context_function(void (*context)(struct context_arg*)) {
-       x_cgo_context_function = context;
-}
-
 // Releases the cgo traceback context.
 void _cgo_release_context(uintptr_t ctxt) {
-       if (ctxt != 0 && x_cgo_context_function != nil) {
+       void (*pfn)(struct context_arg*);
+
+       pfn = _cgo_get_context_function();
+       if (ctxt != 0 && pfn != nil) {
                struct context_arg arg;
 
                arg.Context = ctxt;
-               (*x_cgo_context_function)(&arg);
+               (*pfn)(&arg);
        }
 }

diff --git a/src/runtime/cgo/gcc_libinit.c b/src/runtime/cgo/gcc_libinit.c
index c5b9476380238fc116a87cbaa4b472e5db00fa08..0bdf40a4ca77c956a91f46908070ef39dc7ff971 100644 (file)
--- a/src/runtime/cgo/gcc_libinit.c
+++ b/src/runtime/cgo/gcc_libinit.c
@@ -15,6 +15,9 @@ static pthread_cond_t runtime_init_cond = PTHREAD_COND_INITIALIZER;
 static pthread_mutex_t runtime_init_mu = PTHREAD_MUTEX_INITIALIZER;
 static int runtime_init_done;
 
+// The context function, used when tracing back C calls into Go.
+static void (*cgo_context_function)(struct context_arg*);
+
 void
 x_cgo_sys_thread_create(void* (*func)(void*), void* arg) {
        pthread_t p;
@@ -27,16 +30,30 @@ x_cgo_sys_thread_create(void* (*func)(void*), void* arg) {
 
 uintptr_t
 _cgo_wait_runtime_init_done() {
+       void (*pfn)(struct context_arg*);
+
        pthread_mutex_lock(&runtime_init_mu);
        while (runtime_init_done == 0) {
                pthread_cond_wait(&runtime_init_cond, &runtime_init_mu);
        }
+
+       // TODO(iant): For the case of a new C thread calling into Go, such
+       // as when using -buildmode=c-archive, we know that Go runtime
+       // initialization is complete but we do not know that all Go init
+       // functions have been run. We should not fetch cgo_context_function
+       // until they have been, because that is where a call to
+       // SetCgoTraceback is likely to occur. We are going to wait for Go
+       // initialization to be complete anyhow, later, by waiting for
+       // main_init_done to be closed in cgocallbackg1. We should wait here
+       // instead. See also issue #15943.
+       pfn = cgo_context_function;
+
        pthread_mutex_unlock(&runtime_init_mu);
-       if (x_cgo_context_function != nil) {
+       if (pfn != nil) {
                struct context_arg arg;
 
                arg.Context = 0;
-               (*x_cgo_context_function)(&arg);
+               (*pfn)(&arg);
                return arg.Context;
        }
        return 0;
@@ -49,3 +66,21 @@ x_cgo_notify_runtime_init_done(void* dummy) {
        pthread_cond_broadcast(&runtime_init_cond);
        pthread_mutex_unlock(&runtime_init_mu);
 }
+
+// Sets the context function to call to record the traceback context
+// when calling a Go function from C code. Called from runtime.SetCgoTraceback.
+void x_cgo_set_context_function(void (*context)(struct context_arg*)) {
+       pthread_mutex_lock(&runtime_init_mu);
+       cgo_context_function = context;
+       pthread_mutex_unlock(&runtime_init_mu);
+}
+
+// Gets the context function.
+void (*(_cgo_get_context_function(void)))(struct context_arg*) {
+       void (*ret)(struct context_arg*);
+
+       pthread_mutex_lock(&runtime_init_mu);
+       ret = cgo_context_function;
+       pthread_mutex_unlock(&runtime_init_mu);
+       return ret;
+}

diff --git a/src/runtime/cgo/gcc_libinit_openbsd.c b/src/runtime/cgo/gcc_libinit_openbsd.c
index 07dfcaf660b98d88deec912b31a8f1378b1fd8b6..626bf8adca104a96a742418821d7976ff6b2abd4 100644 (file)
--- a/src/runtime/cgo/gcc_libinit_openbsd.c
+++ b/src/runtime/cgo/gcc_libinit_openbsd.c
@@ -6,6 +6,9 @@
 #include <stdlib.h>
 #include "libcgo.h"
 
+// The context function, used when tracing back C calls into Go.
+static void (*cgo_context_function)(struct context_arg*);
+
 void
 x_cgo_sys_thread_create(void* (*func)(void*), void* arg) {
        fprintf(stderr, "x_cgo_sys_thread_create not implemented");
@@ -14,12 +17,16 @@ x_cgo_sys_thread_create(void* (*func)(void*), void* arg) {
 
 uintptr_t
 _cgo_wait_runtime_init_done() {
+       void (*pfn)(struct context_arg*);
+
        // TODO(spetrovic): implement this method.
-       if (x_cgo_context_function != nil) {
+
+       pfn = _cgo_get_context_function();
+       if (pfn != nil) {
                struct context_arg arg;
 
                arg.Context = 0;
-               (*x_cgo_context_function)(&arg);
+               (*pfn)(&arg);
                return arg.Context;
        }
        return 0;
@@ -29,3 +36,15 @@ void
 x_cgo_notify_runtime_init_done(void* dummy) {
        // TODO(spetrovic): implement this method.
 }
+
+// Sets the context function to call to record the traceback context
+// when calling a Go function from C code. Called from runtime.SetCgoTraceback.
+void x_cgo_set_context_function(void (*context)(struct context_arg*)) {
+       // TODO(iant): Needs synchronization.
+       cgo_context_function = context;
+}
+
+// Gets the context function.
+void (*(_cgo_get_context_function(void)))(struct context_arg*) {
+       return cgo_context_function;
+}

diff --git a/src/runtime/cgo/gcc_libinit_windows.c b/src/runtime/cgo/gcc_libinit_windows.c
index f5c306d49ad6a70f2a6dc8181ef99a4b4a47d818..0824e20ad816838375bbbbd9114c9bb26f3d5a39 100644 (file)
--- a/src/runtime/cgo/gcc_libinit_windows.c
+++ b/src/runtime/cgo/gcc_libinit_windows.c
@@ -70,15 +70,18 @@ _cgo_is_runtime_initialized() {
 
 uintptr_t
 _cgo_wait_runtime_init_done() {
+       void (*pfn)(struct context_arg*);
+
         _cgo_maybe_run_preinit();
        while (!_cgo_is_runtime_initialized()) {
                        WaitForSingleObject(runtime_init_wait, INFINITE);
        }
-       if (x_cgo_context_function != nil) {
+       pfn = _cgo_get_context_function();
+       if (pfn != nil) {
                struct context_arg arg;
 
                arg.Context = 0;
-               (*x_cgo_context_function)(&arg);
+               (*pfn)(&arg);
                return arg.Context;
        }
        return 0;
@@ -98,3 +101,23 @@ x_cgo_notify_runtime_init_done(void* dummy) {
        }
 }
 
+// The context function, used when tracing back C calls into Go.
+static void (*cgo_context_function)(struct context_arg*);
+
+// Sets the context function to call to record the traceback context
+// when calling a Go function from C code. Called from runtime.SetCgoTraceback.
+void x_cgo_set_context_function(void (*context)(struct context_arg*)) {
+       EnterCriticalSection(&runtime_init_cs);
+       cgo_context_function = context;
+       LeaveCriticalSection(&runtime_init_cs);
+}
+
+// Gets the context function.
+void (*(_cgo_get_context_function(void)))(struct context_arg*) {
+       void (*ret)(struct context_arg*);
+
+       EnterCriticalSection(&runtime_init_cs);
+       ret = cgo_context_function;
+       LeaveCriticalSection(&runtime_init_cs);
+       return ret;
+}

diff --git a/src/runtime/cgo/libcgo.h b/src/runtime/cgo/libcgo.h
index 249d052edc142e611356d506512587b0e9678710..01f9e721745300c198858a7700f59e619183cda7 100644 (file)
--- a/src/runtime/cgo/libcgo.h
+++ b/src/runtime/cgo/libcgo.h
@@ -93,7 +93,7 @@ void darwin_arm_init_mach_exception_handler(void);
 struct context_arg {
        uintptr_t Context;
 };
-extern void (*x_cgo_context_function)(struct context_arg*);
+extern void (*(_cgo_get_context_function(void)))(struct context_arg*);
 
 /*
  * TSAN support.  This is only useful when building with