Skip to content

Commit

Permalink
safepoints are required in any lock than may be used with allocations (…
Browse files Browse the repository at this point in the history 
…#40487)

(which is pretty much all locks)
  • Loading branch information
@vtjnash
vtjnash authored Apr 19, 2021
1 parent 6a40cf8 commit 399ec04
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 72 deletions.
5 changes: 4 additions & 1 deletion src/codegen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,10 @@ static const auto jlegal_func = new JuliaFunction{
[](LLVMContext &C) {
Type *T = PointerType::get(T_jlvalue, AddressSpace::CalleeRooted);
return FunctionType::get(T_int32, {T, T}, false); },
nullptr,
[](LLVMContext &C) { return AttributeList::get(C,
Attributes(C, {Attribute::ReadOnly, Attribute::NoUnwind, Attribute::ArgMemOnly}),
AttributeSet(),
None); },
};
static const auto jl_alloc_obj_func = new JuliaFunction{
"julia.gc_alloc_obj",
Expand Down
14 changes: 1 addition & 13 deletions src/dlload.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ JL_DLLEXPORT int jl_dlclose(void *handle) JL_NOTSAFEPOINT
#endif
}

JL_DLLEXPORT void *jl_load_dynamic_library(const char *modname, unsigned flags, int throw_err) JL_NOTSAFEPOINT // (or throw)
JL_DLLEXPORT void *jl_load_dynamic_library(const char *modname, unsigned flags, int throw_err)
{
char path[PATHBUF], relocated[PATHBUF];
int i;
Expand All @@ -178,20 +178,12 @@ JL_DLLEXPORT void *jl_load_dynamic_library(const char *modname, unsigned flags,
if (!GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
(LPCWSTR)(uintptr_t)(&jl_load_dynamic_library),
(HMODULE*)&handle)) {
#ifndef __clang_analyzer__
// Hide the error throwing from the analyser since there isn't a way to express
// "safepoint only when throwing error" currently.
jl_error("could not load base module");
#endif
}
#else
Dl_info info;
if (!dladdr((void*)(uintptr_t)&jl_load_dynamic_library, &info) || !info.dli_fname) {
#ifndef __clang_analyzer__
// Hide the error throwing from the analyser since there isn't a way to express
// "safepoint only when throwing error" currently.
jl_error("could not load base module");
#endif
}
handle = dlopen(info.dli_fname, RTLD_NOW);
#endif
Expand Down Expand Up @@ -274,11 +266,7 @@ JL_DLLEXPORT void *jl_load_dynamic_library(const char *modname, unsigned flags,
#else
const char *reason = dlerror();
#endif
#ifndef __clang_analyzer__
// Hide the error throwing from the analyser since there isn't a way to express
// "safepoint only when throwing error" currently.
jl_errorf("could not load library \"%s\"\n%s", modname, reason);
#endif
}
handle = NULL;

Expand Down
9 changes: 4 additions & 5 deletions src/gc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,9 +480,9 @@ void jl_gc_run_all_finalizers(jl_ptls_t ptls)
run_finalizers(ptls);
}

static void gc_add_finalizer_(jl_ptls_t ptls, void *v, void *f)
static void gc_add_finalizer_(jl_ptls_t ptls, void *v, void *f) JL_NOTSAFEPOINT
{
int8_t gc_state = jl_gc_unsafe_enter(ptls);
assert(ptls->gc_state == 0);
arraylist_t *a = &ptls->finalizers;
// This acquire load and the release store at the end are used to
// synchronize with `finalize_object` on another thread. Apart from the GC,
Expand All @@ -506,15 +506,14 @@ static void gc_add_finalizer_(jl_ptls_t ptls, void *v, void *f)
items[oldlen] = v;
items[oldlen + 1] = f;
jl_atomic_store_release(&a->len, oldlen + 2);
jl_gc_unsafe_leave(ptls, gc_state);
}

JL_DLLEXPORT void jl_gc_add_ptr_finalizer(jl_ptls_t ptls, jl_value_t *v, void *f)
JL_DLLEXPORT void jl_gc_add_ptr_finalizer(jl_ptls_t ptls, jl_value_t *v, void *f) JL_NOTSAFEPOINT
{
gc_add_finalizer_(ptls, (void*)(((uintptr_t)v) | 1), f);
}

JL_DLLEXPORT void jl_gc_add_finalizer_th(jl_ptls_t ptls, jl_value_t *v, jl_function_t *f)
JL_DLLEXPORT void jl_gc_add_finalizer_th(jl_ptls_t ptls, jl_value_t *v, jl_function_t *f) JL_NOTSAFEPOINT
{
if (__unlikely(jl_typeis(f, jl_voidpointer_type))) {
jl_gc_add_ptr_finalizer(ptls, v, jl_unbox_voidpointer(f));
Expand Down
8 changes: 4 additions & 4 deletions src/gf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1368,7 +1368,7 @@ static void invalidate_method_instance(jl_method_instance_t *replaced, size_t ma
}
if (!jl_is_method(replaced->def.method))
return; // shouldn't happen, but better to be safe
JL_LOCK_NOGC(&replaced->def.method->writelock);
JL_LOCK(&replaced->def.method->writelock);
jl_code_instance_t *codeinst = replaced->cache;
while (codeinst) {
if (codeinst->max_world == ~(size_t)0) {
Expand All @@ -1388,13 +1388,13 @@ static void invalidate_method_instance(jl_method_instance_t *replaced, size_t ma
invalidate_method_instance(replaced, max_world, depth + 1);
}
}
JL_UNLOCK_NOGC(&replaced->def.method->writelock);
JL_UNLOCK(&replaced->def.method->writelock);
}

// invalidate cached methods that overlap this definition
static void invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_world, const char *why)
{
JL_LOCK_NOGC(&replaced_mi->def.method->writelock);
JL_LOCK(&replaced_mi->def.method->writelock);
jl_array_t *backedges = replaced_mi->backedges;
if (backedges) {
// invalidate callers (if any)
Expand All @@ -1405,7 +1405,7 @@ static void invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_w
invalidate_method_instance(replaced[i], max_world, 1);
}
}
JL_UNLOCK_NOGC(&replaced_mi->def.method->writelock);
JL_UNLOCK(&replaced_mi->def.method->writelock);
if (why && _jl_debug_method_invalidation) {
jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)replaced_mi);
jl_value_t *loctag = jl_cstr_to_string(why);
Expand Down
14 changes: 7 additions & 7 deletions src/julia.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,8 @@ typedef enum {

JL_DLLEXPORT void jl_gc_collect(jl_gc_collection_t);

JL_DLLEXPORT void jl_gc_add_finalizer(jl_value_t *v, jl_function_t *f);
JL_DLLEXPORT void jl_gc_add_finalizer(jl_value_t *v, jl_function_t *f) JL_NOTSAFEPOINT;
JL_DLLEXPORT void jl_gc_add_ptr_finalizer(jl_ptls_t ptls, jl_value_t *v, void *f) JL_NOTSAFEPOINT;
JL_DLLEXPORT void jl_finalize(jl_value_t *o);
JL_DLLEXPORT jl_weakref_t *jl_gc_new_weakref(jl_value_t *value);
JL_DLLEXPORT jl_value_t *jl_gc_alloc_0w(void);
Expand Down Expand Up @@ -1531,11 +1532,10 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_or_error(jl_module_t *m, jl_sym_t *var
JL_DLLEXPORT jl_value_t *jl_module_globalref(jl_module_t *m, jl_sym_t *var);
// get binding for assignment
JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var, int error);
JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m JL_PROPAGATES_ROOT,
jl_sym_t *var);
JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var);
JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var);
JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var) JL_NOTSAFEPOINT;
JL_DLLEXPORT int jl_binding_resolved_p(jl_module_t *m, jl_sym_t *var) JL_NOTSAFEPOINT;
JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var);
JL_DLLEXPORT int jl_binding_resolved_p(jl_module_t *m, jl_sym_t *var);
JL_DLLEXPORT int jl_is_const(jl_module_t *m, jl_sym_t *var);
JL_DLLEXPORT jl_value_t *jl_get_global(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var);
JL_DLLEXPORT void jl_set_global(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT);
Expand All @@ -1549,7 +1549,7 @@ JL_DLLEXPORT void jl_module_import(jl_module_t *to, jl_module_t *from, jl_sym_t
JL_DLLEXPORT void jl_module_import_as(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname);
JL_DLLEXPORT void jl_module_export(jl_module_t *from, jl_sym_t *s);
JL_DLLEXPORT int jl_is_imported(jl_module_t *m, jl_sym_t *s);
JL_DLLEXPORT int jl_module_exports_p(jl_module_t *m, jl_sym_t *var) JL_NOTSAFEPOINT;
JL_DLLEXPORT int jl_module_exports_p(jl_module_t *m, jl_sym_t *var);
JL_DLLEXPORT void jl_add_standard_imports(jl_module_t *m);
STATIC_INLINE jl_function_t *jl_get_function(jl_module_t *m, const char *name)
{
Expand Down Expand Up @@ -1703,7 +1703,7 @@ enum JL_RTLD_CONSTANT {
#define JL_RTLD_DEFAULT (JL_RTLD_LAZY | JL_RTLD_DEEPBIND)

typedef void *jl_uv_libhandle; // compatible with dlopen (void*) / LoadLibrary (HMODULE)
JL_DLLEXPORT jl_uv_libhandle jl_load_dynamic_library(const char *fname, unsigned flags, int throw_err) JL_NOTSAFEPOINT;
JL_DLLEXPORT jl_uv_libhandle jl_load_dynamic_library(const char *fname, unsigned flags, int throw_err);
JL_DLLEXPORT jl_uv_libhandle jl_dlopen(const char *filename, unsigned flags) JL_NOTSAFEPOINT;
JL_DLLEXPORT int jl_dlclose(jl_uv_libhandle handle) JL_NOTSAFEPOINT;
JL_DLLEXPORT int jl_dlsym(jl_uv_libhandle handle, const char *symbol, void ** value, int throw_err) JL_NOTSAFEPOINT;
Expand Down
7 changes: 3 additions & 4 deletions src/julia_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ void jl_compute_field_offsets(jl_datatype_t *st);
jl_array_t *jl_new_array_for_deserialization(jl_value_t *atype, uint32_t ndims, size_t *dims,
int isunboxed, int hasptr, int isunion, int elsz);
void jl_module_run_initializer(jl_module_t *m);
jl_binding_t *jl_get_module_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var) JL_NOTSAFEPOINT;
jl_binding_t *jl_get_module_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var);
void jl_binding_deprecation_warning(jl_module_t *m, jl_binding_t *b);
extern jl_array_t *jl_module_init_order JL_GLOBALLY_ROOTED;
extern htable_t jl_current_modules JL_GLOBALLY_ROOTED;
Expand Down Expand Up @@ -1043,10 +1043,9 @@ extern void *jl_crtdll_handle;
extern void *jl_winsock_handle;
#endif

void *jl_get_library_(const char *f_lib, int throw_err) JL_NOTSAFEPOINT;
void *jl_get_library_(const char *f_lib, int throw_err);
#define jl_get_library(f_lib) jl_get_library_(f_lib, 1)
JL_DLLEXPORT void *jl_load_and_lookup(const char *f_lib, const char *f_name,
void **hnd) JL_NOTSAFEPOINT;
JL_DLLEXPORT void *jl_load_and_lookup(const char *f_lib, const char *f_name, void **hnd);
JL_DLLEXPORT void *jl_lazy_load_and_lookup(jl_value_t *lib_val, const char *f_name);
JL_DLLEXPORT jl_value_t *jl_get_cfunction_trampoline(
jl_value_t *fobj, jl_datatype_t *result, htable_t *cache, jl_svec_t *fill,
Expand Down
36 changes: 18 additions & 18 deletions src/module.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ static jl_binding_t *new_binding(jl_sym_t *name)
// get binding for assignment
JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var, int error)
{
JL_LOCK_NOGC(&m->lock);
JL_LOCK(&m->lock);
jl_binding_t **bp = (jl_binding_t**)ptrhash_bp(&m->bindings, var);
jl_binding_t *b = *bp;

Expand All @@ -160,7 +160,7 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m JL_PROPAGATES_ROOT,
b->owner = m;
}
else if (error) {
JL_UNLOCK_NOGC(&m->lock);
JL_UNLOCK(&m->lock);
jl_errorf("cannot assign a value to variable %s.%s from module %s",
jl_symbol_name(b->owner->name), jl_symbol_name(var), jl_symbol_name(m->name));
}
Expand All @@ -170,10 +170,11 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m JL_PROPAGATES_ROOT,
b = new_binding(var);
b->owner = m;
*bp = b;
JL_GC_PROMISE_ROOTED(b);
jl_gc_wb_buf(m, b, sizeof(jl_binding_t));
}

JL_UNLOCK_NOGC(&m->lock);
JL_UNLOCK(&m->lock);
return b;
}

Expand Down Expand Up @@ -208,7 +209,7 @@ JL_DLLEXPORT jl_module_t *jl_get_module_of_binding(jl_module_t *m, jl_sym_t *var
// like jl_get_binding_wr, but has different error paths
JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_t *var)
{
JL_LOCK_NOGC(&m->lock);
JL_LOCK(&m->lock);
jl_binding_t **bp = _jl_get_module_binding_bp(m, var);
jl_binding_t *b = *bp;

Expand All @@ -218,7 +219,7 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_
b->owner = m;
}
else {
JL_UNLOCK_NOGC(&m->lock);
JL_UNLOCK(&m->lock);
jl_binding_t *b2 = jl_get_binding(b->owner, b->name);
if (b2 == NULL || b2->value == NULL)
jl_errorf("invalid method definition: imported function %s.%s does not exist",
Expand All @@ -239,7 +240,7 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_
jl_gc_wb_buf(m, b, sizeof(jl_binding_t));
}

JL_UNLOCK_NOGC(&m->lock);
JL_UNLOCK(&m->lock);
return b;
}

Expand Down Expand Up @@ -583,33 +584,33 @@ JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var)

JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var)
{
JL_LOCK_NOGC(&m->lock);
JL_LOCK(&m->lock);
jl_binding_t *b = (jl_binding_t*)ptrhash_get(&m->bindings, var);
JL_UNLOCK_NOGC(&m->lock);
JL_UNLOCK(&m->lock);
return b != HT_NOTFOUND && (b->exportp || b->owner==m);
}

JL_DLLEXPORT int jl_module_exports_p(jl_module_t *m, jl_sym_t *var) JL_NOTSAFEPOINT
JL_DLLEXPORT int jl_module_exports_p(jl_module_t *m, jl_sym_t *var)
{
JL_LOCK_NOGC(&m->lock);
JL_LOCK(&m->lock);
jl_binding_t *b = _jl_get_module_binding(m, var);
JL_UNLOCK_NOGC(&m->lock);
JL_UNLOCK(&m->lock);
return b != HT_NOTFOUND && b->exportp;
}

JL_DLLEXPORT int jl_binding_resolved_p(jl_module_t *m, jl_sym_t *var) JL_NOTSAFEPOINT
JL_DLLEXPORT int jl_binding_resolved_p(jl_module_t *m, jl_sym_t *var)
{
JL_LOCK_NOGC(&m->lock);
JL_LOCK(&m->lock);
jl_binding_t *b = _jl_get_module_binding(m, var);
JL_UNLOCK_NOGC(&m->lock);
JL_UNLOCK(&m->lock);
return b != HT_NOTFOUND && b->owner != NULL;
}

JL_DLLEXPORT jl_binding_t *jl_get_module_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var) JL_NOTSAFEPOINT
JL_DLLEXPORT jl_binding_t *jl_get_module_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var)
{
JL_LOCK_NOGC(&m->lock);
JL_LOCK(&m->lock);
jl_binding_t *b = _jl_get_module_binding(m, var);
JL_UNLOCK_NOGC(&m->lock);
JL_UNLOCK(&m->lock);
return b == HT_NOTFOUND ? NULL : b;
}

Expand All @@ -626,7 +627,6 @@ JL_DLLEXPORT void jl_set_global(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *va
JL_TYPECHK(jl_set_global, module, (jl_value_t*)m);
JL_TYPECHK(jl_set_global, symbol, (jl_value_t*)var);
jl_binding_t *bp = jl_get_binding_wr(m, var, 1);
JL_GC_PROMISE_ROOTED(bp);
jl_checked_assignment(bp, val);
}

Expand Down
38 changes: 18 additions & 20 deletions src/runtime_ccall.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ using namespace llvm;
static std::map<std::string, void*> libMap;
static jl_mutex_t libmap_lock;
extern "C"
void *jl_get_library_(const char *f_lib, int throw_err) JL_NOTSAFEPOINT
void *jl_get_library_(const char *f_lib, int throw_err)
{
void *hnd;
if (f_lib == NULL)
return jl_RTLD_DEFAULT_handle;
#ifdef _OS_WINDOWS_
Expand All @@ -40,23 +39,22 @@ void *jl_get_library_(const char *f_lib, int throw_err) JL_NOTSAFEPOINT
if (strcmp(f_lib, JL_LIBJULIA_DL_LIBNAME) == 0)
return jl_libjulia_handle;
#endif
JL_LOCK_NOGC(&libmap_lock);
JL_LOCK(&libmap_lock);
// This is the only operation we do on the map, which doesn't invalidate
// any references or iterators.
void **map_slot = &libMap[f_lib];
JL_UNLOCK_NOGC(&libmap_lock);
hnd = jl_atomic_load_acquire(map_slot);
if (hnd != NULL)
return hnd;
// We might run this concurrently on two threads but it doesn't matter.
hnd = jl_load_dynamic_library(f_lib, JL_RTLD_DEFAULT, throw_err);
if (hnd != NULL)
jl_atomic_store_release(map_slot, hnd);
void *hnd = *map_slot;
if (hnd == NULL) {
hnd = jl_load_dynamic_library(f_lib, JL_RTLD_DEFAULT, throw_err);
if (hnd != NULL)
*map_slot = hnd;
}
JL_UNLOCK(&libmap_lock);
return hnd;
}

extern "C" JL_DLLEXPORT
void *jl_load_and_lookup(const char *f_lib, const char *f_name, void **hnd) JL_NOTSAFEPOINT
void *jl_load_and_lookup(const char *f_lib, const char *f_name, void **hnd)
{
void *handle = jl_atomic_load_acquire(hnd);
if (!handle)
Expand Down Expand Up @@ -210,11 +208,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 jl_mutex_t trampoline_lock; // for accesses to the cache and freelist

static void *trampoline_freelist;

static void *trampoline_alloc() // lock taken by caller
static void *trampoline_alloc() JL_NOTSAFEPOINT // lock taken by caller
{
const int sz = 64; // oversized for most platforms. todo: use precise value?
if (!trampoline_freelist) {
Expand All @@ -235,6 +233,7 @@ static void *trampoline_alloc() // lock taken by caller
#endif
errno = last_errno;
void *next = NULL;
assert(sz < jl_page_size);
for (size_t i = 0; i + sz <= jl_page_size; i += sz) {
void **curr = (void**)((char*)mem + i);
*curr = next;
Expand Down Expand Up @@ -272,6 +271,8 @@ static void trampoline_deleter(void **f)
JL_UNLOCK_NOGC(&trampoline_lock);
}

typedef void *(*init_trampoline_t)(void *tramp, void **nval) JL_NOTSAFEPOINT;

// Use of `cache` is not clobbered in JL_TRY
JL_GCC_IGNORE_START("-Wclobbered")
extern "C" JL_DLLEXPORT
Expand All @@ -282,7 +283,7 @@ jl_value_t *jl_get_cfunction_trampoline(
// call-site constants:
htable_t *cache, // weakref htable indexed by (fobj, vals)
jl_svec_t *fill,
void *(*init_trampoline)(void *tramp, void **nval),
init_trampoline_t init_trampoline,
jl_unionall_t *env,
jl_value_t **vals)
{
Expand Down Expand Up @@ -339,11 +340,8 @@ jl_value_t *jl_get_cfunction_trampoline(
((void**)result)[1] = (void*)fobj;
}
if (!permanent) {
void *ptr_finalizer[2] = {
(void*)jl_voidpointer_type,
(void*)&trampoline_deleter
};
jl_gc_add_finalizer(result, (jl_value_t*)&ptr_finalizer[1]);
jl_ptls_t ptls = jl_get_ptls_states();
jl_gc_add_ptr_finalizer(ptls, result, (void*)(uintptr_t)&trampoline_deleter);
((void**)result)[2] = (void*)cache;
((void**)result)[3] = (void*)nval;
}
Expand Down

0 comments on commit 399ec04

Please sign in to comment.