diff --git a/src/runtime_ccall.cpp b/src/runtime_ccall.cpp index 461d98ac9b437..307acc9c28cd1 100644 --- a/src/runtime_ccall.cpp +++ b/src/runtime_ccall.cpp @@ -210,9 +210,11 @@ extern "C" JL_DLLEXPORT char *jl_format_filename(const char *output_pattern) } +static jl_mutex_t trampoline_lock; // for accesses to the cache and freelist + static void *trampoline_freelist; -static void *trampoline_alloc() +static void *trampoline_alloc() // lock taken by caller { const int sz = 64; // oversized for most platforms. todo: use precise value? if (!trampoline_freelist) { @@ -245,7 +247,7 @@ static void *trampoline_alloc() return tramp; } -static void trampoline_free(void *tramp) +static void trampoline_free(void *tramp) // lock taken by caller { *(void**)tramp = trampoline_freelist; trampoline_freelist = tramp; @@ -260,17 +262,18 @@ static void trampoline_deleter(void **f) f[0] = NULL; f[2] = NULL; f[3] = NULL; + JL_LOCK_NOGC(&trampoline_lock); if (tramp) trampoline_free(tramp); if (fobj && cache) ptrhash_remove((htable_t*)cache, fobj); if (nval) free(nval); + JL_UNLOCK_NOGC(&trampoline_lock); } // Use of `cache` is not clobbered in JL_TRY JL_GCC_IGNORE_START("-Wclobbered") -// TODO: need a thread lock around the cache access parts of this function extern "C" JL_DLLEXPORT jl_value_t *jl_get_cfunction_trampoline( // dynamic inputs: @@ -284,6 +287,7 @@ jl_value_t *jl_get_cfunction_trampoline( jl_value_t **vals) { // lookup (fobj, vals) in cache + JL_LOCK_NOGC(&trampoline_lock); if (!cache->table) htable_new(cache, 1); if (fill != jl_emptysvec) { @@ -295,6 +299,7 @@ jl_value_t *jl_get_cfunction_trampoline( } } void *tramp = ptrhash_get(cache, (void*)fobj); + JL_UNLOCK_NOGC(&trampoline_lock); if (tramp != HT_NOTFOUND) { assert((jl_datatype_t*)jl_typeof(tramp) == result_type); return (jl_value_t*)tramp; @@ -347,10 +352,12 @@ jl_value_t *jl_get_cfunction_trampoline( free(nval); jl_rethrow(); } + JL_LOCK_NOGC(&trampoline_lock); tramp = trampoline_alloc(); ((void**)result)[0] = tramp; tramp = init_trampoline(tramp, nval); ptrhash_put(cache, (void*)fobj, result); + JL_UNLOCK_NOGC(&trampoline_lock); return result; } JL_GCC_IGNORE_STOP diff --git a/test/threads_exec.jl b/test/threads_exec.jl index 691fca2fb2afa..87b3edc7c8025 100644 --- a/test/threads_exec.jl +++ b/test/threads_exec.jl @@ -471,7 +471,6 @@ end function test_thread_cfunction() # ensure a runtime call to `get_trampoline` will be created - # TODO: get_trampoline is not thread-safe (as this test shows) fs = [ Core.Box() for i in 1:1000 ] @noinline cf(f) = @cfunction $f Float64 () cfs = Vector{Base.CFunction}(undef, length(fs)) @@ -494,11 +493,7 @@ function test_thread_cfunction() @test sum(ok) == 10000 end if cfunction_closure - if nthreads() == 1 - test_thread_cfunction() - else - @test_broken "cfunction trampoline code not thread-safe" - end + test_thread_cfunction() end # Compare the two ways of checking if threading is enabled.