godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.72k stars 570 forks source link

get_world() causing crash possibly leak! #568

Open AbanoubNassem opened 3 years ago

AbanoubNassem commented 3 years ago

calling get_world() from KinematicBody causing crash possibly leak! , currently I'm calling it in the _ready method :

    void Hero::_ready() {
        space_state = get_world()->get_direct_space_state();
    }

simple demo : https://github.com/AbanoubNassem/godot-cpp-cmake 

operating system : MacOS BigSur 11.4

Process:               Godot [6302]
Path:                  /Applications/Godot.app/Contents/MacOS/Godot
Identifier:            org.godotengine.godot
Version:               3.3.2 (3.3.2)
Code Type:             X86-64 (Native)
Parent Process:        Godot [89971]
Responsible:           Godot [89971]
User ID:               501

Date/Time:             2021-06-02 14:18:33.186 +0400
OS Version:            macOS 11.4 (20F71)
Report Version:        12
Anonymous UUID:        5A760DD8-1F25-4DF6-F63B-F44F00976AC4

Sleep/Wake UUID:       E33078B4-1206-4335-B5E6-5B38E6F7FD46

Time Awake Since Boot: 140000 seconds
Time Since Wake:       1200 seconds

System Integrity Protection: disabled

Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGABRT)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000122946aa0
Exception Note:        EXC_CORPSE_NOTIFY

VM Regions Near 0x122946aa0:
    __LINKEDIT                  119bd4000-119bda000    [   24K] rw-/rwx SM=NUL  /System/Library/Frameworks/CoreMediaIO.framework/Versions/A/Resources/VDC.plugin/Contents/MacOS/VDC
--> 
    CG image                    122ece000-122ed3000    [   20K] rw-/rwx SM=COW  

Application Specific Information:
abort() called

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib          0x00007fff2030992e __pthread_kill + 10
1   libsystem_pthread.dylib         0x00007fff203385bd pthread_kill + 263
2   libsystem_c.dylib               0x00007fff2028d411 abort + 120
3   org.godotengine.godot           0x000000010494e73c 0x10494c000 + 10044
4   libsystem_platform.dylib        0x00007fff2037dd7d _sigtramp + 29
5   ???                             000000000000000000 0 + 0
6   org.godotengine.godot           0x00000001072ef508 0x10494c000 + 43660552
7   org.godotengine.godot           0x00000001062d4395 0x10494c000 + 26772373
8   org.godotengine.godot           0x000000010628a728 0x10494c000 + 26470184
9   org.godotengine.godot           0x000000010495cd2a 0x10494c000 + 68906
10  org.godotengine.godot           0x0000000104963421 0x10494c000 + 95265
11  libdyld.dylib                   0x00007fff20353f5d start + 1

Thread 1:
0   libsystem_pthread.dylib         0x00007fff20334420 start_wqthread + 0

Thread 2:
0   libsystem_pthread.dylib         0x00007fff20334420 start_wqthread + 0

Thread 3:
0   libsystem_kernel.dylib          0x00007fff20305cde __psynch_cvwait + 10
1   libsystem_pthread.dylib         0x00007fff20338e49 _pthread_cond_wait + 1298
2   libc++.1.dylib                  0x00007fff202a1d72 std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 18
3   org.godotengine.godot           0x0000000107508c9b _IP_ResolverPrivate::_thread_function(void*) + 171
4   org.godotengine.godot           0x000000010747f02f 0x10494c000 + 45297711
5   org.godotengine.godot           0x000000010747f567 0x10494c000 + 45299047
6   libsystem_pthread.dylib         0x00007fff203388fc _pthread_start + 224
7   libsystem_pthread.dylib         0x00007fff20334443 thread_start + 15

Thread 4:
0   libsystem_kernel.dylib          0x00007fff20305cde __psynch_cvwait + 10
1   libsystem_pthread.dylib         0x00007fff20338e49 _pthread_cond_wait + 1298
2   libc++.1.dylib                  0x00007fff202a1d72 std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 18
3   org.godotengine.godot           0x00000001070b5bfb 0x10494c000 + 41327611
4   org.godotengine.godot           0x000000010747f02f 0x10494c000 + 45297711
5   org.godotengine.godot           0x000000010747f567 0x10494c000 + 45299047
6   libsystem_pthread.dylib         0x00007fff203388fc _pthread_start + 224
7   libsystem_pthread.dylib         0x00007fff20334443 thread_start + 15

Thread 5:: AMCP Logging Spool
0   libsystem_kernel.dylib          0x00007fff203032f6 semaphore_wait_trap + 10
1   com.apple.audio.caulk           0x00007fff2864b8da caulk::mach::semaphore::wait_or_error() + 16
2   com.apple.audio.caulk           0x00007fff28638836 caulk::semaphore::timed_wait(double) + 110
3   com.apple.audio.caulk           0x00007fff28638784 caulk::concurrent::details::worker_thread::run() + 30
4   com.apple.audio.caulk           0x00007fff28638502 void* caulk::thread_proxy<std::__1::tuple<caulk::thread::attributes, void (caulk::concurrent::details::worker_thread::*)(), std::__1::tuple<caulk::concurrent::details::worker_thread*> > >(void*) + 45
5   libsystem_pthread.dylib         0x00007fff203388fc _pthread_start + 224
6   libsystem_pthread.dylib         0x00007fff20334443 thread_start + 15

Thread 6:: com.apple.audio.IOThread.client
0   libsystem_kernel.dylib          0x00007fff203032ba mach_msg_trap + 10
1   libsystem_kernel.dylib          0x00007fff2030362c mach_msg + 60
2   com.apple.audio.CoreAudio       0x00007fff21d378f5 HALB_MachPort::SendSimpleMessageWithSimpleReply(unsigned int, unsigned int, int, int&, bool, unsigned int) + 111
3   com.apple.audio.CoreAudio       0x00007fff21bda3ed invocation function for block in HALC_ProxyIOContext::HALC_ProxyIOContext(unsigned int, unsigned int) + 3367
4   com.apple.audio.CoreAudio       0x00007fff21d760c4 HALB_IOThread::Entry(void*) + 72
5   libsystem_pthread.dylib         0x00007fff203388fc _pthread_start + 224
6   libsystem_pthread.dylib         0x00007fff20334443 thread_start + 15

Thread 7:: com.apple.NSEventThread
0   libsystem_kernel.dylib          0x00007fff203032ba mach_msg_trap + 10
1   libsystem_kernel.dylib          0x00007fff2030362c mach_msg + 60
2   com.apple.CoreFoundation        0x00007fff204318c5 __CFRunLoopServiceMachPort + 316
3   com.apple.CoreFoundation        0x00007fff2042ff7b __CFRunLoopRun + 1332
4   com.apple.CoreFoundation        0x00007fff2042f380 CFRunLoopRunSpecific + 567
5   com.apple.AppKit                0x00007fff22ce068a _NSEventThread + 124
6   libsystem_pthread.dylib         0x00007fff203388fc _pthread_start + 224
7   libsystem_pthread.dylib         0x00007fff20334443 thread_start + 15

Thread 0 crashed with X86 Thread State (64-bit):
  rax: 0x0000000000000000  rbx: 0x0000000114bb6e00  rcx: 0x00007ffeeb2b0e78  rdx: 0x0000000000000000
  rdi: 0x0000000000000103  rsi: 0x0000000000000006  rbp: 0x00007ffeeb2b0ea0  rsp: 0x00007ffeeb2b0e78
   r8: 0x00007fff80630068   r9: 0x0000000000000000  r10: 0x0000000114bb6e00  r11: 0x0000000000000246
  r12: 0x0000000000000103  r13: 0x00007ffeeb2b1710  r14: 0x0000000000000006  r15: 0x0000000000000016
  rip: 0x00007fff2030992e  rfl: 0x0000000000000246  cr2: 0x00007fff87050228

Logical CPU:     0
Error Code:      0x02000148
Trap Number:     133

Thread 0 instruction stream not available.

Thread 0 last branch register state not available.
Calinou commented 3 years ago

@AbanoubNassem Can you reproduce this in your project by doing the same operation in GDScript?

Also, it would be useful to have a minimal reproduction project available for troubleshooting.

AbanoubNassem commented 3 years ago

@Calinou

I have made a simple project to test : https://github.com/AbanoubNassem/godot-cpp-cmake/

please follow instructions there to build godot-cpp and library , after the build finishes, the output library should be inside the game folder , open godot project and link the library , after that just run the scene try to close the window it will hung then crash

Zylann commented 3 years ago

Looking at the crash dump, I dont see anything useful. It would be interesting if you could compile the bindings and your library in debug mode, and run your project using an actual debugger because it would likely be able to point out the full detailed call stack, rather than a cryptic crash (there are sooooo many people asking for help without doing any of that, it's kinda crazy).

Also a crash and a leak are different things, you reported a crash, but I dont see how you deduced the existence of any leak.

About the code:

namespace Library {
    class World : public godot::Spatial {
    GODOT_CLASS(World, Spatial)

I'm surprised this even compiles. Spatial here is used without the godot:: namespace prefixed. In this macro it is used here: https://github.com/godotengine/godot-cpp/blob/fda7ddd158b2d2ef6d7af7d509694415ec81d615/include/core/Godot.hpp#L110

You don't use godot:: here either https://github.com/AbanoubNassem/godot-cpp-cmake/blob/29f543ca757c85008edacb0ba029113344a7e6d1/library/src/sources/World.cpp#L10 Even though register_method is not even inherited, it's a regular function in the godot:: namespace, yet somehow we can use it without the namespace Oo

Now, based on what the title of this issue claims, I did a simple test myself by just adding the following CustomSpatial to a scene:

#include <core/Godot.hpp>
#include <gen/Spatial.hpp>

namespace yolo {

class CustomSpatial : public godot::Spatial {
    GODOT_CLASS(CustomSpatial, Spatial)
public:
    CustomSpatial();
    ~CustomSpatial();

    void _init();

    void _ready();

    static void _register_methods();

private:
};

} // namespace yolo

/////////////////////////

#include "custom_spatial.h"
#include <gen/World.hpp>

namespace yolo {

CustomSpatial::CustomSpatial() {
}

CustomSpatial::~CustomSpatial() {
}

void CustomSpatial::_init() {
}

void CustomSpatial::_ready() {
    godot::Godot::print("Ready");
    auto world = get_world();
}

void CustomSpatial::_register_methods() {
    godot::register_method("_ready", &CustomSpatial::_ready);
}

} // namespace yolo

No crash occurred. However I was surprised to see that if I omit godot:: myself, compilation works fine... which isn't normal, isn't it? Because the problem is, if I name my class World too... isn't it going to conflict with godot::World? Anyways, I could not reproduce this by solely doing auto world = get_world() inside _ready(). I know there is your project but then it means the problem originates from something else (I'm too tired to investigate more atm).

AbanoubNassem commented 3 years ago

@Zylann

Looking at the crash dump, I dont see anything useful. It would be interesting if you could compile the bindings and your library in debug mode, and run your project using an actual debugger because it would likely be able to point out the full detailed call stack, rather than a cryptic crash (there are sooooo many people asking for help without doing any of that, it's kinda crazy).

well I completely understand you, but not all of us are 100% working on games and on Godot perticaulary , so for most of us it's test/try the Engine and see, if it fits our needs , and that is the case for me , I wanted to see if Godot and NativeScript are the best fit for me , without going to all this hustle!

Also a crash and a leak are different things, you reported a crash, but I dont see how you deduced the existence of any leak.

I completely understand that as well, the reason I stated it might be a leak , is this issue : https://github.com/godotengine/godot-cpp/issues/85 , also when I close the app , it hung and take a bit to close like trying to free some memory .

I have added godot:: to everywhere it should have been added , changed the World to CustomWorld everywhere , and still does the same thing : https://youtu.be/zvDQMKnY75E

AbanoubNassem commented 3 years ago

@Zylann @Calinou

I have attached a debug to it , and here is the result :

handle_crash: Program crashed with signal 11
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] 1   libsystem_platform.dylib            0x00007fff2037dd7d _sigtramp + 29
[2] 2   ???                                 0x0000000000000000 0x0 + 0
[3] Object::can_translate_messages() const
[4] SceneTree::get_root() const
[5] SceneTree::get_root() const
[6] DefaultAllocator::alloc(unsigned long)
[7] DirAccess* DirAccess::_create_builtin<DirAccessOSX>()
[8] 8   libdyld.dylib                       0x00007fff20353f5d start + 1
[9] 9   ???                                 0x0000000000000009 0x0 + 9
-- END OF BACKTRACE --

is this has any useful information ?

Zylann commented 3 years ago

It seems like this isn't at all from get_world then, more like some directory operation (which is strange cuz I see none of that in your code). Now I suspect version mismatch or some OSX-specific issue. But hard to tell what the call stack is really doing, or even where these stack frames belong to. SceneTree::get_root() is an inline getter so I dont understand why can_translate_messages or alloc surrounds it... usually debug builds show the associated cpp file as well, it seems incomplete... is this the entire call stack? Is your library and bindings built in debug mode as well? (if they are not, a debugger not only won't show the files, but it will also likely show optimized out call stacks which might not make sense)

About the namespace, I dont think it is the cause of the issue, rather a strange curiosity going on.

AbanoubNassem commented 3 years ago

@Zylann unfortunately it seems to crash outside the binding , Error :EXC_BAD_ACCESS (code=EXC_I386_GPFLT)

here is the disassembly : https://gist.github.com/AbanoubNassem/a8cf6610d683a773808cb00dad0762f5

it crashes on line 87 , but I can't get any much information out of it 🤷🏻‍♂️

Zylann commented 3 years ago

Still doesnt help (at least for me, I can't read that). What I would do here is to compile litterally everything in debug mode and run it through a debugger. Is this what you did here? Because ideally the goal would be to get the full detailed C++ call stack with file names and lines, not a disassembly (also I don't have OSX to test this on the same platform)