free can be used to cause a segfault entirely from rust

[lilizoey]

use godot::{engine::Engine, prelude::*};
struct Testing;

#[gdextension]
unsafe impl ExtensionLibrary for Testing {}

#[derive(GodotClass)]
#[class(init, base = Node)]
pub struct Foo {
    #[base]
    base: Base<Node>,
}

#[godot_api]
impl NodeVirtual for Foo {
    fn ready(&mut self) {
        if Engine::singleton().is_editor_hint() {
            return;
        }

        let node = Node::new_alloc();
        let node2 = node.share();
        node2.free();

        self.base.add_child(node);
    }
}

Adding a node of class Foo will cause a segfault when the game runs.

[lilizoey]

Another related pattern that is harder to guard against:

let child = Node::new_alloc();
let mut node = Node::new_alloc();
let node2 = node.share();

let node_deref = &mut *node;
node2.free();
// segfaults
node_deref.add_child(child.share());
child.free();

[lilizoey]

The most straightforward solution here would be to make deref check for liveness (as a best effort), and to make free an unsafe function. Hopefully people can mostly use reference counting and queue_free for managing this. But if people use free a lot then this would add a bunch of unsafe to the code.

But again, objects that dont inherit from Node and RefCounted are kinda rare in general. We have a bunch of them in our integration tests, but i dont think that's a representative sample.

I'm not sure if there's a good way to make a completely safe abstraction for this.

[lilizoey]

Some conversation about this issue happening in this discord thread.

[lilizoey]

queue_free can also segfault from rust:

#[derive(GodotClass)]
#[class(init, base = Node)]
pub struct Foo {
    #[base]
    base: Base<Node>,
    node: Option<&'static mut Node>,
    i: u64,
}

#[godot_api]
impl NodeVirtual for Foo {
    fn ready(&mut self) {
        if Engine::singleton().is_editor_hint() {
            return;
        }

        let node = Node::new_alloc();

        self.add_child(node.share());

        self.node = Some(&mut *Box::leak(Box::new(node)));
    }

    fn process(&mut self, _delta: f64) {
        let i = self.i;
        if let Some(node) = &mut self.node {
            match i {
                0 => node.queue_free(),
                1 => {
                    let child = Node::new_alloc();
                    node.add_child(child);
                }
                _ => (),
            }
        }

        self.i += 1;
    }
}