Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Lock cfunction trampoline cache and freelist accesses. #39621

Merged
merged 2 commits into from
Feb 13, 2021
Merged

Lock cfunction trampoline cache and freelist accesses. #39621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01cb47f
Lock cfunction trampoline cache and freelist accesses.
maleadt Feb 11, 2021
9583c6b
test cfunction closure with threads
vchuravy Feb 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/runtime_ccall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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:
Expand All @@ -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) {
Expand All @@ -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;
Expand Down Expand Up @@ -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
7 changes: 1 addition & 6 deletions test/threads_exec.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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.
Expand Down