-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Ensure jl_get_excstack can't read from concurrently running tasks #30899
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
Conversation
|
Looks like the FreeBSD build slave ran out of disk space. |
|
CI passes and this is a minor internal fix which forbids erroneous user code, so I'll merge this in a day or two if there's no objections. |
9005be2 to
073a7d1
Compare
This might not be a major problem right now, but could become a source of crashes once PARTR lands.
073a7d1 to
bd78876
Compare
|
OSX failure is #30949 |
|
Could it be that this broke (testing of) functions that currently use with using Test, LinearAlgebra
function trmul!(a, b)
Threads.@threads for i = 1:length(a)
a[i] *= b
end
return a
end
a = randn(Float64, 1000);
b = randn(Float64);
a2 = copy(a);
trmul!(a2, b);
rmul!(a, b);
a2 == a # true
trmul!(a2, b) == rmul!(a, b) # sometimes works, returning true, sometimes hangs
@test rmul!(copy(a), b) == trmul!(copy(a), b) # always hangsand upon |
|
Bump, could this be looked at before 1.2 becomes final? I think it would be bad to break the current multithreading facilities? |
|
@vtjnash, can you take a look? |
|
@Jutho superficially your error doesn't look related to this change. Is there a reason to believe that |
|
Probably not, I don't know or understand anything about the internals and the C code. I just noticed that this PR was one of the last to modify stackwalk.c. I assumed the |
|
No problems. The stack trace looks like it's hitting some other problem maybe related to some larger concurrency improvements / refactorings that Jameson and Jeff have merged lately. But I've not had time to follow those developments closely so that's just a guess. |
A minor tweak extracted from #29901 to prevent users from accidentally calling
catch_stackon a concurrently running task. I don't think this is something which makes sense to do and the current implementation would cause crashes once PARTR lands.