No helpful message on C++ stack overflow

[lemonlambda]

// This causes a crash no matter if the node even exists in a scene that's how good it is.

use godot::classes::*;
use godot::prelude::*;

#[derive(GodotClass)]
#[class(base = Node)]
pub struct Crasher {
    child: Gd<CrasherChild>,

    base: Base<Node>,
}

#[godot_api]
impl INode for Crasher {
    fn init(base: Base<Node>) -> Self {
        Self {
            child: CrasherChild::new_alloc(),

            base,
        }
    }
}

#[derive(GodotClass)]
#[class(base = Node)]
pub struct CrasherChild {
    parent: Gd<Crasher>,

    base: Base<Node>,
}

#[godot_api]
impl INode for CrasherChild {
    fn init(base: Base<Node>) -> Self {
        Self {
            parent: Crasher::new_alloc(),

            base,
        }
    }
}

This causes godot 4.3 to crash even if the node(s) don't exist in the scene. A warning or an error for this would be nice if possible.

[Yarwin]

This example causes never-ending loop which actually causes the crash (Crasher creates CrasherChild and vice versa); It should happen in the editor as well.

This issue will be rendered null after #1051 will be merged (i.e in 0.3.0 release) – it will remove Export implementations for Gd<T> and DynGd<T, D>. For now on one can use Option<Gd<T>> and OnReady<Gd>` (no breaking changes in user facing API are planned for now).

See also: #892

[lilizoey]

i dont see why removing the Export impl for Gd would do anything here, the child/parent fields aren't exported.

[Yarwin]

Oh, my bad, the discord example included the #[export].

Same use case appeared in the now hidden repo.

The question is if anything can be done about that – should we permit using Gd<T> as a property at all? (IMO not, if user wants to create never-ending loop and crash the engine then we should allow that 🤔)

[lilizoey]

i dont think we can prevent it though. rust doesn't have any non-intrusive way to stop a type from being used as a field in a struct.

[lemonlambda]

i dont think we can prevent it though. rust doesn't have any non-intrusive way to stop a type from being used as a field in a struct.

You can use a proc-macro to detect if it's self-referential but this requires both structs to have this proc-macro. This could be done with GodotClass. This is probably over-cumbersome though.

[lilizoey]

i dont think we can prevent it though. rust doesn't have any non-intrusive way to stop a type from being used as a field in a struct.

You can use a proc-macro to detect if it's self-referential but this requires both structs to have this proc-macro. This could be done with GodotClass. This is probably over-cumbersome though.

yeah but it also doesn't work with indirection.

the above code is also fine as long as it's not cyclic, and there are cases where you might want that. which also increases the complexity of detecting this, either needing an opt-out or cycle detection, and it's not easy to reliably share state between macros invocations.

we might be able to detect a cycle and error out early, or at least give a better error message. but we can't in general detect all infinite loops.

[Bromeon]

@lemonlambda You're writing the godot-rust equivalent of this code*:
_{* which I find quite elegant}

#[derive(Default)]
struct Parent {
    child: Box<Child>,
}

#[derive(Default)]
struct Child {
    parent: Box<Parent>,
}

fn main() {
    let _crash = Parent::default();
}

I don't really see why/how we should try to prevent this?

[lemonlambda]

@lemonlambda You're writing the godot-rust equivalent of this code*: * which I find quite elegant

#[derive(Default)]
struct Parent {
child: Box,
}

#[derive(Default)]
struct Child {
parent: Box,
}

fn main() {
let _crash = Parent::default();
}

I don't really see why/how we should try to prevent this?

At least with-in rust you will get a stack overflow error, with godot you will just get a crash with no meaningful error. I guess that's my main problem.

[Bromeon]

That's a Godot problem though, no? The equivalent GDScript probably has the same behavior?

Or what should the Rust bindings concretely do to detect and act on such cases?

[lemonlambda]

I have no clue. Maybe it is a godot issue instead and should report it as so.

[Bromeon]

Can you try reproducing it in GDScript?

[lilizoey]

i was curious so i checked

my code:

# in node1.gd
class_name Node1
extends Node

var node: Node2 = Node2.new()

# in node2.gd
class_name Node2
extends Node

var node: Node1 = Node1.new()

after adding to a scene and then running that scene, the game immediately stops running and a stack overflow error is displayed in the editor:

[Bromeon]

Thanks for checking! It looks like the GDScript runtime has its own stack, which it can check. It even displayes the size (1024).

---

If I run my example in a #[test], I get:

thread 'test_fn' has overflowed its stack
fatal runtime error: stack overflow

But this is best-effort, not guaranteed, see here. It may also vary between platforms.

If I run the same example (nothing with Godot!) in an #[itest] -- custom integration test, Rust code hosted by the Godot engine -- I instead get:

Process finished with exit code 139 (interrupted by signal 11:SIGSEGV)

which is probably a consequence of our code being hosted by Godot, a C++ application with different handling of stack overflow.

---

To sum up:

• Pure Rust code may print a nice message on stack overflow.
• The C++ configuration for Godot does not seem to handle/detect stack overflows at the moment.
• A lot of this is best-effort and likely non-portable.

In other words, I still don't see what godot-rust can realistically do against this. Maybe Godot can improve something and make stack overflows visible in its crash handler. But that would then be an issue to be taken up with the engine itself.

So unless someone comes up with a concrete idea how we can handle this better, I'd tend to close this issue.

[Bromeon]

I brought this up in Godot's RocketChat, there were some interesting answers.

It was suggested that sigaltstack may be used to define an alternative stack for such purposes. Details would still need to be researched, there's also probably some platform-dependent aspects playing into it, if this should be supported across platforms.

This is out of scope for godot-rust though, as it needs to be configured in the host application (Godot). If someone wants to pursue this, I'd recommend opening a Godot proposal with concrete requirements + maybe even ideas for design.

Thanks for reporting, I learned quite a few things! 🙂