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.

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

add a healthy dose of locks to generic functions #17250

Merged
merged 2 commits into from
Jul 5, 2016
Merged

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Jul 2, 2016

also fix a couple of recently-introduced data-flow ordering bugs in typeinf

fix #17059
fix #16524

@@ -1888,12 +1890,14 @@ static void jl_add_linfo_root(jl_lambda_info_t *li, jl_value_t *val)
size_t rlen = jl_array_dim0(m->roots);
for(size_t i=0; i < rlen; i++) {
if (jl_array_ptr_ref(m->roots,i) == val) {
jl_mutex_unlock(&li->def->writelock);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JL_UNLOCK?

@yuyichao yuyichao added the domain:multithreading Base.Threads and related functionality label Jul 2, 2016
@@ -1878,6 +1879,7 @@ static void jl_add_linfo_root(jl_lambda_info_t *li, jl_value_t *val)
if (jl_is_leaf_type(val) || jl_is_bool(val) || jl_is_symbol(val))
return;
JL_GC_PUSH1(&val);
JL_LOCK(&li->def->writelock);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&m->writelock

@yuyichao
Copy link
Contributor

yuyichao commented Jul 2, 2016

Nice!! #16524 probably needs a few more atomic operations to make sure the write is visibly in a consistent state for the reader but this should be good enough for now.

@yuyichao
Copy link
Contributor

yuyichao commented Jul 3, 2016

I rebased this to resolve the conflict with #17116 (also resolved #17250 (comment)) There seems to be many other issues according to the CI though...

@@ -1176,13 +1180,16 @@ void *jl_get_llvmf(jl_tupletype_t *tt, bool getwrapper, bool getdeclarations)
}
if (linfo->jlcall_api == 2) {
// normally we don't generate native code for these functions, so need an exception here
if (linfo->functionObjectsDecls.functionObject == NULL)
to_function(linfo);
// Yes, this leaks lots of memory. No, I don't care.
Copy link
Contributor

@tkelman tkelman Jul 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, why not? This isn't the kind of comment we want to have sitting around in the codebase.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@vtjnash vtjnash merged commit a8bb4ee into master Jul 5, 2016
@vtjnash vtjnash deleted the jn/thread-lock branch July 5, 2016 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:multithreading Base.Threads and related functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nondetermintistic codegen segfault with @threads Accessing TypeMap is not threadsafe
3 participants