godotengine / godot

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

Loading & unloading GDNative EditorPlugin crashes editor #47325

Open derekdai opened 3 years ago

derekdai commented 3 years ago

Godot version: 3.2 ~ 3.3.rc.custom_build.6da21f472

OS/device including version:

Issue description: Unload and reload GDNative EditorPlugin can crashes Godot editor.

The crash occurs if EditorPlugin with reloadable=on, any activity which trigger plugin unloading may leave some sort of invalid references (through pointer) owned by unloaded plugin in Godot editor. If editor access any of these resources, segmentation fault.

https://github.com/godotengine/godot-cpp/issues/535

I also found that the handle pass into gdotot_nativescript_init and godot_nativescript_terminate are different. Since there are not much document about the interface of GDNative, I'm not sure it is ok or not.

Steps to reproduce:

  1. Build the minimal GDNative EditorPlugin
  2. Run Godot Editor
  3. Make sure the reloadable=on in .gdnlib
  4. Enable the plugin from Project Settings (now plugin been loaded)
  5. Switch to another window (unfocus Godot editor, the plugin should be unloaded now)
  6. Switch back to Godot editor. If no crash, Alt-Tab multiple times till it crash (segmentation fault)

The step 6 can replace with exit from Godot editor, it will terminated with segmentation fault.

Minimal reproduction project: Use cpp/simple project of godot-cpp as base

// MainScreen.hpp
#include <Godot.hpp>
#include <String.hpp>
#include <Control.hpp>
#include <EditorPlugin.hpp>

using namespace godot;

class MainScreen : public EditorPlugin {
    GODOTCLASS(MainScreen, EditorPlugin)

public:
    static void _registermethods();

    bool has_main_screen();

    Ref<Texture> get_plugin_icon();

    String get_plugin_name();
};

```cpp
// MainScreen.cpp
#include <Godot.hpp>
#include <EditorInterface.hpp>

#include "MainScreen.hpp"

using namespace godot;

void MainScreen::_register_methods() {
    register_method("has_main_screen", &MainScreen::has_main_screen);
    register_method("get_plugin_icon", &MainScreen::get_plugin_icon);
    register_method("get_plugin_name", &MainScreen::get_plugin_name);
}

bool MainScreen::has_main_screen()
{
    return true;
}

godot::String MainScreen::get_plugin_name()
{
    return String("Main Screen");   
}

Ref<Texture> MainScreen::get_plugin_icon()
{
  return get_editor_interface()->get_base_control()->get_icon("Node", "EditorIcons");
}

extern "C" void GDN_EXPORT godot_gdnative_init(godot_gdnative_init_options *o) {
    godot::Godot::gdnative_init(o);
}

extern "C" void GDN_EXPORT godot_nativescript_init(void *handle) {
    godot::Godot::nativescript_init(handle);

    godot::register_tool_class<MainScreen>();
}

extern "C" void GDN_EXPORT godot_gdnative_terminate(godot_gdnative_terminate_options *o) {
    godot::Godot::gdnative_terminate(o);
}
# mainscreen.gdnlib
[general]

singleton=false
load_once=true
symbol_prefix="godot_"
reloadable=true

[entry]

X11.64="res://bin/libmain_screen.so"

[dependencies]

X11.64=[  ]
# mainscreen.gdns
[gd_resource type="NativeScript" load_steps=2 format=2]

[ext_resource path="res://addons/mainscreen/mainscreen.gdnlib" type="GDNativeLibrary" id=1]

[resource]
class_name = "MainScreen"
library = ExtResource( 1 )
parasyte commented 2 years ago

My understanding from #19686 and #32580 is that reloadable must be disabled with a GDNative EditorPlugin or tool script. I just stumbled upon this open issue while investigating a similar issue I was experiencing.

If there is anything that will improve the situation, though, I'm all for it! For instance, a reloadable EditorPlugin could be removed from the scene tree when unloaded and fully initialized when reloaded.

git2013vb commented 1 year ago

Happen with Godot 4 beta 17 too. Debian 11 I use Git Plugin https://godotengine.org/asset-library/asset/1581 and when I remove it from file tab godot close aromatically without errors