godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.66k stars 20.11k forks source link

"Condition 'p_elem->_root!=this' is true." errors when adding nodes from a thread #8691

Closed supercom32 closed 6 years ago

supercom32 commented 7 years ago

Hi everyone,

I seem to be randomly getting these "Condition 'p_elem->_root!=this' is true." errors. When they happen, it causes my "Errors" tab to fill up with messages. I had a look in core/self_list.h:59 to see what is up, but I'm afraid it's all too technical for me. (^_^);

Has anyone else experienced this before, and/or know why it would be generated? Or that matter, what the error actually means? What's strange is that it happens randomly, so I have no real way of investigating further.

akien-mga commented 7 years ago

Does it still happen in the current master branch, and if so, can you provide a minimal project that reproduces the issue and/or describe the steps to reproduce?

reduz commented 7 years ago

I think I havent seen this error in a while did anyone?

akien-mga commented 7 years ago

I think it has been fixed already.

mysticfall commented 6 years ago

I'm having the same issue with a pretty recent build (68f277477b5a5f3003405e36c6b6853720cb91de) of the master branch:

ERROR: remove: Condition ' p_elem->_root != this ' is true.
   At: core/self_list.h:77.

Should we reopen this issue?

akien-mga commented 6 years ago

Now that it's reopened let's use this issue, but next time please open a new one instead. This issue has been fixed more than half a year ago, and AFAIK it's not present in the 3.0 stable version. So the issue you're having now, though showing the same symptoms, is likely a recently introduced bug and thus not present since May 2017 as reported here.

mysticfall commented 6 years ago

Sorry if I revived an irrelevant issue. I thought it was the same one, since the message was identical and there was no mention of the actual commit that fixed the problem in this thread.

I'll report a separate issue in future, if I encounter a similar case as this.

akien-mga commented 6 years ago

No problem :) But yeah, those messages are quite generic and can be triggered by many things (typically a GUI element in the editor that starts making changes too early while its elements haven't been initialized yet), so we might have different causes leading to the same error message.

mysticfall commented 6 years ago

It seems that I can reproduce this issue quite often. Is there anything I can do to help track down the cause, like putting some debug statement inside the editor code?

If so, please let me know and I'll test it as soon as I can.

vnen commented 6 years ago

@mysticfall that can be tracked only if you put Godot into an external debugger and add a breakpoint in the referred line. Then you can check the call stack to have a hint of where it starts giving problems.

mysticfall commented 6 years ago

@vnen Thanks for the information. I wish I knew enough of C++ to debug this :disappointed:

organicpencil commented 6 years ago

Most recently I got this in the 2.1 branch when calling astar.get_point_path after mistakenly making duplicate connections with bidirectional=true. It still spit out the path just fine. That was weird to debug.

Probably obscure issue with user code. I will pray for you.

MortimerMcMire commented 6 years ago

setting bidirectional to false fixed that error for me

the error is triggered whenever a new object has to calculate a path between two astar nodes that were connected to each other twice (easy to do if you run code on each node to connect it to every other adjacent node, bidirectional = true)

it's probably triggered from other things too, but I think in this case it's worth breaking out the error into a more descriptive "You're connecting your astar points twice, try setting bidirectional to false"

mysticfall commented 6 years ago

In my case, I don't use any path finding features but still got the error. But if most others who have the same symptom only experience it in that context, then I agree we should change the title to something more descriptive.

I'll try to find a way to reproduce it more reliably and open another issue in that case.

ianscottcamp commented 6 years ago

@akien-mga I had this error back in v 2.1 and the problem only worsened for me in 3.0. For me the issue occurs when adding nodes to the current running scene tree from a thread

supercom32 commented 6 years ago

@ianscottcamp: I also had the same issue. That is, when adding multiple nodes to the current running scene from a thread, sometimes I got these errors. Seems thread related to be honest.

ianscottcamp commented 6 years ago

@supercom32 yeah i narrowed it down to add_child from a thread which using call deferred got rid of the p_elem error and revealed other errors that weren't being thrown before. Fixing those new errors helped reduce random crashes i was getting. Using call deferred did give me lag though as i am adding around 1000 nodes at a time. I reverted back to not using it as i could live with p elem error over the bad lag on world generation. My game is procedural gen exploration

ianscottcamp commented 6 years ago

@mortimermcmire Yeah a bunch of people mentioned astar was not thread safe. I started using astar but could not easily control connections in my game since mine is an infinite expanding proc generated terrain. I ended up making my own pathfinding algo to suit my needs. Are you using astar with a thread?

On Tue, Jul 10, 2018, 3:49 PM MortimerMcMire notifications@github.com wrote:

I get the error again when I add astar nodes, and even more errors when objects try to find connections.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/8691#issuecomment-403960807, or mute the thread https://github.com/notifications/unsubscribe-auth/ANlKIK_wqj57DqbGTGwxYle4MPndU0I4ks5uFRNLgaJpZM4NTglu .

reduz commented 6 years ago

@ianscottcamp Calling add_child from a thread will most likely cause unfixable instability. I suggest you still use call_deferred, but do something like calling a function of your own and pass an argument with an array containing all the objects you want to add, so it's just one call.

reduz commented 6 years ago

@ianscottcamp alternatively you can use servers directly instead of nodes, those work perfect if you call functions from multiple threads

reduz commented 6 years ago

AStar and Navigation are problematic, because while it may make sense for many to use it from a thread, it's an algorithm that requires a lot of working memory (which is optimized by storing this in the structure itself). This makes it thread-unsafe.

This would need some discussion on how it could be made work better from threads without the significant performance cost of allocating those structures all the time, but I suggest opening a new issue.

I will close this though, as the original issue never reappeared.

efhilton commented 4 years ago

I'm seeing this in 3.2.2. I have a thread that creates/adds Nodes. I've been adding locks everywhere that my Thread executes. But the problem is still there. I'm using C#.

ZackingIt commented 3 years ago

Ditto -- I have the variant

ERROR: remove: Condition "p_elem->_root != this" is true.
   At: ./core/self_list.h:84

pop up every so often and basically makes Godot unusable.

Calinou commented 3 years ago

@ZackingIt Please open a new issue with a minimal reproduction project attached.

ZackingIt commented 3 years ago

I wanted to add that disabling physics multi-threading seems resolve this issue, OR making sure to connect a signal as "deferred" if you are using multi-threading and that signal around a physics process (i.e., _body_entered).

db0 commented 3 years ago

I wanted to add that disabling physics multi-threading seems resolve this issue, OR making sure to connect a signal as "deferred" if you are using multi-threading and that signal around a physics process (i.e., _body_entered).

Ooof, thanks for tipping me off to this. I was wondering why my game started crashing a lot! Disabling multi-threading solved it.

Firty commented 2 years ago

I needed to add more than a thousand nodes to my tree, and this was causing a lock on the main thread, in an attempt to get around this, I put a secondary thread to do this process. After several times using randomly the problem is triggered. I use threads to do a lot of things, and this was the first time I had a problem with this. This link contains the log resulting from the problem. I created a Debug system in my project where each function called records time, Script name, function name and the Thread it recorded.

https://www.dropbox.com/s/2ep3dp9b8twnctu/godot.log?dl=0

Sorry, I'm Brazilian and English is not my strong point.

Godot Engine v3.5.beta1.official.b9b23d222

My simple solution for now will be to not use Secondary Threads with add_child.

SolsticeProjekt commented 2 years ago

I am getting lots of these as well.

E 0:00:05.529 remove: Condition "p_elem->_root != this" is true. <C++ Source> ./core/self_list.h:80 @ remove()

All the thread's doing is setting the positions of Bodies and Sprites. All the add_child's, which I've seen mentioned here, are in ready().

Coder2195Text commented 1 year ago

Im having this error in Godot 4

s-p-n commented 1 year ago

Im having this error in Godot 4

Me too. I am still investigating where the problem is coming from exactly. I am using threads. It's hard to tell if the problem is fixed though, because it happens sometimes (our favorite kind of error, right?)

Anyway, I am unsafely assuming the nature of problems that happen sometimes is probably thread-related. Godot 4 doesn't really give error messages for bad things that happen in threads, which makes things more difficult.

I tried changing some code to take the add_child calls out of the thread and move them to the function that gets called once the thread is finished. The errors seem to have went away for now.

eimfach commented 1 year ago

Interacting with the active scene tree is NOT thread-safe. Make sure to use mutexes when sending data between threads. If you want to call functions from a thread, the call_deferred function may be used:

https://docs.godotengine.org/en/stable/tutorials/performance/threads/thread_safe_apis.html#doc-thread-safe-apis

jcarlosrc commented 1 year ago

I can confirm it appears on Godot 4.0.2 when adding children on a separate thread. Seems to be solved with call_deferred.