Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid RIDs detected upon opening the Editor #64199

Closed
Rubonnek opened this issue Aug 10, 2022 · 14 comments
Closed

Invalid RIDs detected upon opening the Editor #64199

Rubonnek opened this issue Aug 10, 2022 · 14 comments

Comments

@Rubonnek
Copy link
Member

Rubonnek commented Aug 10, 2022

Godot version

3.x at a7559fc

System information

Arch Linux

Issue description

By just opening the Editor you'll see several invalid RID error messages in the terminal.

These messages started appearing in 3.x after #55764 was merged.

Steps to reproduce

  1. Create a new project and close the Editor.
  2. In a terminal, open the project with godot -e --path /path/to/project
  3. You'll see the following invalid RIDs error messages in the terminal:
ERROR: Invalid RID.
   at: free (servers/visual/visual_server_raster.cpp:69)
ERROR: Invalid RID.
   at: free (servers/visual/visual_server_raster.cpp:69)
ERROR: Invalid RID.
   at: free (servers/visual/visual_server_raster.cpp:69)
ERROR: Invalid RID.
   at: free (servers/visual/visual_server_raster.cpp:69)

Minimal reproduction project

An empty new project is enough. using GLES2 suffices.

@Rubonnek
Copy link
Member Author

CC @tinmanjuggernaut

@lawnjelly
Copy link
Member

The other problem as I mentioned in the PR, is that this error message does not distinguish between a NULL RID and a RID not found.

@TokisanGames
Copy link
Contributor

TokisanGames commented Aug 10, 2022

We can add the differentiation between null and not found.

@Rubonnek The PR is working as intended. You are building a development branch. The error means someone introduced a bug in the editor that is attempting to free invalid RIDs. If you build with tracking rids, it will hopefully tell you where in the engine code the bug is coming from.

scons rids=tracked_handles

See #54907

@lawnjelly
Copy link
Member

lawnjelly commented Aug 10, 2022

I'm not getting any RID errors with a rids=tracked_handles build with latest 3.x, just for info.

Is this a fork PumpUpTheJams#1 and not the official 3.x?

@Rubonnek
Copy link
Member Author

@Rubonnek The PR is working as intended.

I never meant to imply it wasn't. My intention was simply to expose the error messages because these weren't showing up in the Editor.

I'm not getting any RID errors with a rids=tracked_handles build with latest 3.x, just for info.

I just rebuilt the latest 3.x at 850c5cc withrids=tracked_handles as well and I still see those messages.

The NULL RID seems to be related to a GIProbe.

Here's a backtrace:

(gdb) bt
#0  VisualServerRaster::free (this=0x7054a10, p_rid=...) at servers/visual/visual_server_raster.cpp:69
#1  0x00000000047bc985 in VisualServerWrapMT::free (this=0x7314d20, p1=...) at servers/visual/visual_server_wrap_mt.h:673
#2  0x0000000003d2bc31 in GIProbe::~GIProbe (this=0x74bd6a0, __in_chrg=<optimized out>) at scene/3d/gi_probe.cpp:530
#3  0x00000000018bd85e in memdelete<Object> (p_class=0x74bd6a0) at ./core/os/memory.h:115
#4  0x00000000049bb662 in ClassDB::class_get_default_property_value (p_class=..., p_property=..., r_valid=0x7fffffffc5a8) at core/class_db.cpp:1376
#5  0x0000000003146469 in get_documentation_default_value (p_class_name=..., p_property_name=..., r_default_value_valid=@0x7fffffffc5a8: false) at editor/doc/doc_data.cpp:251
#6  0x0000000003147025 in DocData::generate (this=0x891c510, p_basic_types=true) at editor/doc/doc_data.cpp:346
#7  0x0000000002d84653 in EditorHelp::generate_doc () at editor/editor_help.cpp:1520
#8  0x0000000002e03c7e in EditorNode::EditorNode (this=0x89141c0) at editor/editor_node.cpp:5858
#9  0x00000000018b748b in Main::start () at main/main.cpp:1953
#10 0x000000000185ee43 in main (argc=2, argv=0x7fffffffde98) at platform/x11/godot_x11.cpp:58

The GIProbe was instanced so that ClassDB could get a default property value, See ClassDB::class_get_default_property_value on stack frame #3 below:

(gdb) bt
#0  GIProbe::GIProbe (this=0x89d4e20) at scene/3d/gi_probe.cpp:514
#1  0x000000000395028a in ClassDB::creator<GIProbe> () at ./core/class_db.h:137
#2  0x00000000049b5b95 in ClassDB::instance (p_class=...) at core/class_db.cpp:520
#3  0x00000000049bb4b2 in ClassDB::class_get_default_property_value (p_class=..., p_property=..., r_valid=0x7fffffffc5a8) at core/class_db.cpp:1359
#4  0x0000000003146469 in get_documentation_default_value (p_class_name=..., p_property_name=..., r_default_value_valid=@0x7fffffffc5a8: false) at editor/doc/doc_data.cpp:251
#5  0x0000000003147025 in DocData::generate (this=0x8926170, p_basic_types=true) at editor/doc/doc_data.cpp:346
#6  0x0000000002d84653 in EditorHelp::generate_doc () at editor/editor_help.cpp:1520
#7  0x0000000002e03c7e in EditorNode::EditorNode (this=0x8927880) at editor/editor_node.cpp:5858
#8  0x00000000018b748b in Main::start () at main/main.cpp:1953
#9  0x000000000185ee43 in main (argc=2, argv=0x7fffffffde98) at platform/x11/godot_x11.cpp:58

I suppose the other invalid RIDs I see may be caused by ClassDB as well.

@TokisanGames
Copy link
Contributor

I never meant to imply it wasn't.

It's fine. :)

I just rebuilt the latest 3.x at 850c5cc withrids=tracked_handles as well and I still see those messages.

You're always supposed to see the messages when there are RID related bugs. With tracked handles, you're supposed to see more messages, as shown in that PR I sent. Didn't you get a dump of the RID database on exit? The backtrace is helpful.

I suppose the other invalid RIDs I see may be caused by ClassDB as well.

No, the errors are caused when some class requests a resource from a server, then they invalidate it (often by freeing it), then they try to free it again. The errors come as a result of a call to free() as shown below.

The NULL RID seems to be related to a GIProbe.
#2 0x0000000003d2bc31 in GIProbe::~GIProbe (this=0x74bd6a0, __in_chrg=) at scene/3d/gi_probe.cpp:530

Indeed, GIProbe must be mishandling it's gi_probe RID somewhere. Line 530:

GIProbe::~GIProbe() {
	VS::get_singleton()->free(gi_probe);
}

I don't see how the RID is used or accessed on a cursory glance. At the moment I'm working on a PR to expand the error messages. Tracking down the actual cause of the bug in GIProbe might take some time by me or others.

Are you using GLES2? I get these errors on GLES2 but not 3 on Win11. Here's my new error message, meaning the passed RID is null, presumably from ~GIProbe().

ERROR: VisualServer attempted to free an invalid RID.
   at: (servers/visual/visual_server_raster.cpp:69)

@lawnjelly I do not get the RID tracking database either. Scons reports WARNING: Building with RIDs as tracked handles.

@lawnjelly
Copy link
Member

I do not get the RID tracking database either. Scons reports WARNING: Building with RIDs as tracked handles.

That is because it isn't finding any leaks, it doesn't output anything if no leaks are detected. Maybe I'll change it so it reports "no leaks detected" rather than no output to show it is working. Can't remember why I disabled this output.

Maybe there is no leak - either an object is deleted earlier, or never created.

@TokisanGames
Copy link
Contributor

@Rubonnek Please confirm that you are using GLES2.

Here's the likely problem.

GIProbe::GIProbe()
	gi_probe = RID_PRIME(VS::get_singleton()->gi_probe_create());
	
RID RasterizerStorageGLES2::gi_probe_create()
	return RID();

GIProbe::~GIProbe() {
	VS::get_singleton()->free(gi_probe);

GIProbe isn't supported by GLES2. But since it's loading it anyway, if GIProbe checks whether the RID is valid before freeing, that should fix the errors. I tried it and am down to 3 instead of 4 errors. There are likely 3 more classes doing the same thing.

@lawnjelly
Copy link
Member

The answer seems to be simple - there is no GI probe in GLES2? So the RID is returned as NULL. There is no problem, it is just #55764 reports it as a problem.

@TokisanGames
Copy link
Contributor

Since we consider attempting to free an invalid RID as a problem (Re: #53374), classes like GI Probe should take better care of their RIDs and only free them if valid.

@Rubonnek
Copy link
Member Author

Didn't you get a dump of the RID database on exit?

I did not -- not when using GLES2 at least.

The answer seems to be simple - there is no GI probe in GLES2?

This is it. Both of the projects I tested in I was using GLES2. I don't get the invalid RIDs when using GLES3.

@TokisanGames
Copy link
Contributor

TokisanGames commented Aug 10, 2022

I've found all four instances. They all behave the same way. No more errors on GLES2 startup. #64235.
I also differentiated the free(RID) error messages: #64234.

@TokisanGames
Copy link
Contributor

@akien-mga This should have been closed by #64235.

@lawnjelly
Copy link
Member

Closed by #64235 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants