-
-
Notifications
You must be signed in to change notification settings - Fork 106
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
Thread safety #100
Comments
There was a bug found in the thread safety of SmartNodes, the latest release (0.9.5) contains the fix for it |
This happens with 0.9.5-alpha (from the release download) |
From your code above it looks like you are creating a new SmartNode per function call, ie no cross-thread access for a given node, is that true? or is the code above simplified? |
That's the function being called each time (it's just copy pasted directly.) There's an FFI boundary to get to that function, which I should have mentioned in the original ticket (sorry, just forgot to include it.) No cross thread access for a given node, or at least, there shouldn't be afaict. // Setup an empty array filled with 0.0s
let mut result =
Vec::with_capacity(x_size as usize * y_size as usize);
result.resize(result.capacity(), 0.0);
// RAII mutex lock guard
let _lock = MUTEX.lock();
unsafe {
// Calls the previously pasted function
ffi::Generate2dNoise(
x_offset.into(),
y_offset.into(),
x_size.into(),
y_size.into(),
seed.into(),
octaves.into(),
frequency,
result.as_mut_ptr(),
);
}
// Return the result / drop the mutex lock guard
return result; |
If this is only showing up for me, I can try to repro it in just C++ later, but was curious if there were any known issues or anything |
Is your |
They're all 32x32 (so xSize * ySize = 1024) I'll see if I can come up with a better set of repro steps |
Can you see what line of code it is crashing on? |
It's hard to pinpoint exactly right now because can only get disassembly after it's linked etc, but it looks like it's somewhere in this: |
Could you show the assembly where it crashes? Can you get the crash on a debug build for more info? |
Yep! I'll see if I can setup the debug build etc, if I can't get that working I'll try setting up a purely C++ repro using the same compiler tooling etc later (probably won't have time this week for that one tho) |
Here's the disassembly! haven't seen it crash with a debug build yet, though: https://gist.github.com/jihiggins/164ed62dd843e59c736f5ba3b208d1cf#file-disassembly-asm-L48 |
Thanks. In |
(and then no problems next run, etc) |
Really weird that it only happens occasionally... in your code can you check that 'fnSimplex` is not nullptr. Did you try this?
|
Yep, this latest set of stuff is with that set to true. I'll add an assert for the fnSimplex check |
(Sorry in advance if this turns out to be some wonky FFI thing haha) |
I've ran into the exact same problem. I was able to partially track it down, for some reason Either way for now I'm simply re-creating the base generator and re-setting the source in derived generators when |
That does sound weird. Are you sure the generator you are setting the source for and the source are both not null? Can you show your workaround I'm not 100% sure what you mean |
It seems like with the latest release this problem doesn't happen anymore (afaict, I've had it running without the mutex for awhile now), so it's probably fine to just close out this issue :u Thanks again! |
If I call this function from multiple threads at the same time (it's currently using 8 worker threads):
I get random segfaults every once in awhile (it seems to happen pretty early on, or not at all)
![image](https://user-images.githubusercontent.com/9329131/190175603-87cf1f4b-979d-486a-a1fe-445c73866442.png)
Relevant part of the callstack:
Using a global mutex around that function solves the issue, so it seems to be something concurrency related.
(Is there any obvious issue with how I'm using the library / is FastNoise2 actually meant to be threadsafe?)
The text was updated successfully, but these errors were encountered: